Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a27bd3dc authored by Lee Shombert's avatar Lee Shombert
Browse files

Perform SQL comment scanning with a manual parser

SQL strings are scanned to find the (first) SQL command and to
characterize the statement.  This change replaces a regular expression
parser with a hand-crafted parser.

Custom unit tests indicate that the new parser is 10x faster than the
regex implementation.  Native memory allocation done by the Java
Matcher class is eliminated.

Additional unit tests have been added to test the scanner.

Test : atest
 * FrameworksCoreTests:android.database

Flag: android.database.sqlite.simple_sql_comment_scanner
Bug: 329118560

Change-Id: I3124dd7f57147607a07b42cfa711e54a81348b10
parent 60da1b02
Loading
Loading
Loading
Loading
+65 −5
Original line number Diff line number Diff line
@@ -22,6 +22,7 @@ import android.compat.annotation.UnsupportedAppUsage;
import android.content.ContentValues;
import android.content.Context;
import android.content.OperationApplicationException;
import android.database.sqlite.Flags;
import android.database.sqlite.SQLiteAbortException;
import android.database.sqlite.SQLiteConstraintException;
import android.database.sqlite.SQLiteDatabase;
@@ -1609,7 +1610,7 @@ public class DatabaseUtils {
     * Comments either start with "--" and run to the end of the line or are C-style block
     * comments.  The function returns null if a prefix could not be found.
     */
    private static String getSqlStatementPrefixExtended(String sql) {
    private static String getSqlStatementPrefixExtendedRegex(String sql) {
        Matcher m = sPrefixPattern.matcher(sql);
        if (m.lookingAt()) {
            return m.group(PREFIX_GROUP_NUM).toUpperCase(Locale.ROOT);
@@ -1618,6 +1619,61 @@ public class DatabaseUtils {
        }
    }

    /**
     * Return the index of the first character past comments and whitespace.  -1 is returned if
     * a comment is malformed.
     */
    private static int getSqlStatementPrefixOffset(String s) {
        final int limit = s.length() - 2;
        if (limit < 0) return -1;
        int i = 0;
        while (i < limit) {
            final char c = s.charAt(i);
            if (c <= ' ') {
                // This behavior conforms to String.trim(), which is used by the legacy Android
                // SQL prefix logic.  This test is not unicode-aware.  Notice that it accepts the
                // null character as whitespace even though the null character will terminate the
                // SQL string in native code.
                i++;
            } else if (c == '-') {
                if (s.charAt(i+1) != '-') return i;
                i = s.indexOf('\n', i+2);
                if (i < 0) return -1;
                i++;
            } else if (c == '/') {
                if (s.charAt(i+1) != '*') return i;
                i++;
                do {
                    i = s.indexOf('*', i+1);
                    if (i < 0) return -1;
                    i++;
                } while (s.charAt(i) != '/');
                i++;
            } else {
                return i;
            }
        }
        return -1;
    }

    /**
     * Scan past leading comments without using the Java regex routines.
     */
    private static String getSqlStatementPrefixExtendedNoRegex(String sql) {
        int n = getSqlStatementPrefixOffset(sql);
        if (n < 0) {
            // Bad comment syntax.
            return null;
        }
        final int end = sql.length();
        if (n > end) {
            // Bad scanning.  This indicates a programming error.
            return null;
        }
        final int eos = Math.min(n+3, end);
        return sql.substring(n, eos);
    }

    /**
     * Return the extended statement type for the SQL statement.  This is not a public API and it
     * can return values that are not publicly visible.
@@ -1663,12 +1719,16 @@ public class DatabaseUtils {
     * @hide
     */
    public static int getSqlStatementTypeExtended(@NonNull String sql) {
        if (Flags.simpleSqlCommentScanner()) {
            return categorizeStatement(getSqlStatementPrefixExtendedNoRegex(sql), sql);
        } else {
            int type = categorizeStatement(getSqlStatementPrefixSimple(sql), sql);
            if (type == STATEMENT_COMMENT) {
            type = categorizeStatement(getSqlStatementPrefixExtended(sql), sql);
                type = categorizeStatement(getSqlStatementPrefixExtendedRegex(sql), sql);
            }
            return type;
        }
    }

    /**
     * Convert an extended statement type to a public SQL statement type value.
+8 −0
Original line number Diff line number Diff line
@@ -16,3 +16,11 @@ flag {
     description: "Permit updates to TEMP tables in read-only transactions"
     bug: "317993835"
}

flag {
     name: "simple_sql_comment_scanner"
     namespace: "system_performance"
     is_fixed_read_only: true
     description: "Scan SQL comments by hand instead of with a regex"
     bug: "329118560"
}
+5 −1
Original line number Diff line number Diff line
@@ -78,7 +78,8 @@ public class DatabaseUtilsTest {
        final int sel = STATEMENT_SELECT;
        assertEquals(sel, getSqlStatementType("SELECT"));
        assertEquals(sel, getSqlStatementType("  SELECT"));
        assertEquals(sel, getSqlStatementType(" \n SELECT"));
        assertEquals(sel, getSqlStatementType(" \n\r\f\t SELECT"));
        assertEquals(sel, getSqlStatementType(" \n\r\f\t SEL"));

        final int upd = STATEMENT_UPDATE;
        assertEquals(upd, getSqlStatementType("UPDATE"));
@@ -95,6 +96,9 @@ public class DatabaseUtilsTest {
        assertEquals(othr, getSqlStatementType("SE LECT"));
        assertEquals(othr, getSqlStatementType("-- cmt\n SE"));
        assertEquals(othr, getSqlStatementType("WITH"));
        assertEquals(othr, getSqlStatementType("-"));
        assertEquals(othr, getSqlStatementType("--"));
        assertEquals(othr, getSqlStatementType("*/* foo */ SEL"));

        // Verify that leading line-comments are skipped.
        assertEquals(sel, getSqlStatementType("-- cmt\n SELECT"));