From a755f21f996c8e02f9e933ce10ff1dfacb79e1a3 Mon Sep 17 00:00:00 2001 From: Magnus Persson <1431685+magnusp@users.noreply.github.com> Date: Thu, 22 Oct 2026 21:11:11 +0200 Subject: [PATCH] fix(jdbc): keep column origin aligned with the protocol width (#100) The parser mapped the Nth select item to the Nth result column without comparing the two widths. A `*` expansion that SQLite narrows, such as a USING or NATURAL join, or that the parser cannot expand at all, shifted every later column onto the wrong table. Discard the resolution when the expansion width differs from `StmtResult.cols`, so a misalignment reports an unknown origin instead of a wrong one. Load table columns through `pragma_table_xinfo` and exclude hidden columns, which keeps generated columns in the expansion. Drop the right-side duplicate of a USING or NATURAL join column while expanding `*`. --- .../libsql/jdbc/LibsqlResultSetMetaData.java | 8 +- .../libsql/jdbc/SqlTableReference.java | 147 +++++++++++++----- .../jdbc/LibsqlResultSetMetaDataTest.java | 82 +++++++++- .../libsql/jdbc/SqlTableReferenceTest.java | 64 +++++++- 4 files changed, 259 insertions(+), 42 deletions(-) diff --git a/libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/LibsqlResultSetMetaData.java b/libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/LibsqlResultSetMetaData.java index bd0f6a7..9d139c6 100644 --- a/libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/LibsqlResultSetMetaData.java +++ b/libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/LibsqlResultSetMetaData.java @@ -416,7 +416,7 @@ private SqlTableReference.Resolution resolveColumnOrigins() throws SQLException } return new SqlTableReference.TableDefinition( info.createSql(), new ArrayList<>(info.columns().keySet())); - }); + }, cols.size()); } columnOrigins = resolved; tableContextResolved = true; @@ -456,8 +456,9 @@ private TableInfo loadTableInfo(SqlTableReference.TableRef reference) throws SQL private TableInfo loadTableInfo(String schema, String table) throws SQLException { String master = schema.equals("temp") ? "sqlite_temp_master" : "sqlite_master"; String query = "SELECT m.name, m.sql, p.name, p.\"notnull\", p.pk " - + "FROM " + master + " m JOIN pragma_table_info(?, ?) p " + + "FROM " + master + " m JOIN pragma_table_xinfo(?, ?) p " + "WHERE m.type IN ('table', 'view') AND m.name COLLATE NOCASE = ? " + + "AND p.hidden != 1 " + "ORDER BY p.cid"; StmtResult result = connection.executeStatement( new Stmt(query, List.of(Value.of(table), Value.of(schema), Value.of(table)))); @@ -471,8 +472,9 @@ private TableInfo loadAttachedTableInfo(String schema, String table) throws SQLE } String quotedSchema = "\"" + actualSchema.replace("\"", "\"\"") + "\""; String query = "SELECT m.name, m.sql, p.name, p.\"notnull\", p.pk " - + "FROM " + quotedSchema + ".sqlite_master m JOIN pragma_table_info(?, ?) p " + + "FROM " + quotedSchema + ".sqlite_master m JOIN pragma_table_xinfo(?, ?) p " + "WHERE m.type IN ('table', 'view') AND m.name COLLATE NOCASE = ? " + + "AND p.hidden != 1 " + "ORDER BY p.cid"; StmtResult result = connection.executeStatement(new Stmt( query, List.of(Value.of(table), Value.of(actualSchema), Value.of(table)))); diff --git a/libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/SqlTableReference.java b/libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/SqlTableReference.java index 2fc01b2..3275ff8 100644 --- a/libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/SqlTableReference.java +++ b/libsql-jdbc/src/main/java/com/github/magnusp/libsql/jdbc/SqlTableReference.java @@ -98,6 +98,22 @@ boolean matchesQualifier(String qualifier) { private record SourceList(List sources, int next) {} + /** + * The sources of a FROM clause together with the column list {@code *} expands to. The star + * list drops the right-side duplicate of a USING or NATURAL join column, as SQLite does. + */ + private record FromClause(List sources, List starLabels, List starTables) { + + static FromClause of(List sources) { + List labels = new ArrayList<>(); + List tables = new ArrayList<>(); + for (Source source : sources) { + appendStarColumns(source, false, labels, tables); + } + return new FromClause(sources, labels, tables); + } + } + private static final Pattern UNQUOTED_IDENTIFIER = Pattern.compile("[A-Za-z_][A-Za-z0-9_$]*"); private static final Set FROM_END = Set.of( @@ -128,6 +144,19 @@ static Resolution resolve(String sql, Schema schema) throws SQLException { .parseStatement(0, tokens.size(), 0, new HashMap<>()); } + /** + * Resolves column origins and discards the resolution when the parser expansion does not match + * the result width. The protocol column list is authoritative for width, and a shifted + * expansion would attribute every later column to the wrong table. + */ + static Resolution resolve(String sql, Schema schema, int columnCount) throws SQLException { + Resolution resolution = resolve(sql, schema); + if (resolution == null || resolution.labels().size() != columnCount) { + return null; + } + return resolution; + } + private static final class Parser { private final List tokens; @@ -233,41 +262,53 @@ private Resolution parseSelect(int start, int end, int depth, Map= 0 ? setOperator : end; - int from = indexOfWord(index, selectEnd, depth, "FROM"); - int listEnd = from >= 0 ? from : selectEnd; - List sources = List.of(); - if (from >= 0) { - int fromEnd = indexOfAnyWord(from + 1, selectEnd, depth, FROM_END); + int fromIndex = indexOfWord(index, selectEnd, depth, "FROM"); + int listEnd = fromIndex >= 0 ? fromIndex : selectEnd; + FromClause from = FromClause.of(List.of()); + if (fromIndex >= 0) { + int fromEnd = indexOfAnyWord(fromIndex + 1, selectEnd, depth, FROM_END); if (fromEnd < 0) { fromEnd = selectEnd; } - List parsed = parseSources(from + 1, fromEnd, depth, scope); - if (parsed == null || parsed.isEmpty()) { + FromClause parsed = parseSources(fromIndex + 1, fromEnd, depth, scope); + if (parsed == null || parsed.sources().isEmpty()) { return null; } - sources = parsed; + from = parsed; } - return parseSelectItems(index, listEnd, depth, sources, scope); + return parseSelectItems(index, listEnd, depth, from, scope); } - private List parseSources(int start, int end, int depth, Map scope) + private FromClause parseSources(int start, int end, int depth, Map scope) throws SQLException { List sources = new ArrayList<>(); + List starLabels = new ArrayList<>(); + List starTables = new ArrayList<>(); + boolean natural = false; int index = start; while (index < end) { SourceList parsed = parseSource(index, end, depth, scope); if (parsed == null) { return null; } - sources.addAll(parsed.sources()); + int starStart = starLabels.size(); + for (Source source : parsed.sources()) { + sources.add(source); + appendStarColumns(source, natural, starLabels, starTables); + } index = parsed.next(); index = skipIndexModifiers(index, end, depth); + if (isWordAt(index, depth, "USING")) { + dropStarColumns(usingColumns(index, end, depth), starStart, starLabels, starTables); + } index = skipJoinCondition(index, end, depth); if (isSymbolAt(index, ",", depth)) { + natural = false; index++; continue; } if (index < end && isJoinStart(index, end, depth)) { + natural = isWordAt(index, depth, "NATURAL"); index = skipToJoin(index, end, depth); if (index < 0) { break; @@ -277,7 +318,7 @@ private List parseSources(int start, int end, int depth, Map scope) @@ -323,7 +364,7 @@ private SourceList parseSource(int start, int end, int depth, Map grouped = parseSources(index + 1, close, innerDepth, scope); + FromClause grouped = parseSources(index + 1, close, innerDepth, scope); if (grouped == null) { return null; } @@ -341,18 +382,11 @@ private SourceList parseSource(int start, int end, int depth, Map labels = new ArrayList<>(); - List tables = new ArrayList<>(); - for (Source source : grouped) { - if (source.knownColumns()) { - labels.addAll(source.columns()); - tables.addAll(source.tables()); - } + return new SourceList(grouped.sources(), index); } return new SourceList( - List.of(new Source(alias, labels, tables, true, null)), index); + List.of(new Source(alias, grouped.starLabels(), grouped.starTables(), true, null)), + index); } String first = identifier(index); @@ -502,7 +536,7 @@ private Resolution parseReturning(int start, int end, int depth, Map sources, Map scope) + int start, int end, int depth, FromClause from, Map scope) throws SQLException { List labels = new ArrayList<>(); List tables = new ArrayList<>(); @@ -563,7 +597,7 @@ private Resolution parseSelectItems( while (index < end) { int comma = indexOfSymbol(index, end, depth, ","); int itemEnd = comma < 0 ? end : comma; - parseItem(index, itemEnd, depth, sources, scope, labels, tables); + parseItem(index, itemEnd, depth, from, scope, labels, tables); if (comma < 0) { break; } @@ -576,7 +610,7 @@ private void parseItem( int start, int end, int depth, - List sources, + FromClause from, Map scope, List labels, List tables) @@ -609,19 +643,15 @@ private void parseItem( } if (expressionEnd - start == 1 && isSymbol(tokens.get(start), "*")) { - for (Source source : sources) { - if (source.knownColumns()) { - labels.addAll(source.columns()); - tables.addAll(source.tables()); - } - } + labels.addAll(from.starLabels()); + tables.addAll(from.starTables()); return; } ColumnRef qualifiedStar = qualifiedStar(start, expressionEnd); if (qualifiedStar != null) { - Source source = - findSource(qualifiedStar.parts().get(qualifiedStar.parts().size() - 1), sources); + Source source = findSource( + qualifiedStar.parts().get(qualifiedStar.parts().size() - 1), from.sources()); if (source != null && source.knownColumns()) { labels.addAll(source.columns()); tables.addAll(source.tables()); @@ -632,7 +662,7 @@ private void parseItem( ColumnRef column = simpleColumn(start, expressionEnd); if (column != null) { labels.add(alias != null ? alias : column.column()); - tables.add(resolveColumn(column, sources)); + tables.add(resolveColumn(column, from.sources())); return; } @@ -850,6 +880,17 @@ private boolean isAlias(int index, int depth) { return true; } + private List usingColumns(int start, int end, int depth) { + if (!isSymbolAt(start + 1, "(", depth)) { + return List.of(); + } + int close = matchingParen(start + 1); + if (close < 0 || close > end) { + return List.of(); + } + return identifiers(start + 2, close, depth + 1); + } + private List identifiers(int start, int end, int depth) { List identifiers = new ArrayList<>(); int index = start; @@ -1036,6 +1077,40 @@ private static String upper(String value) { return value.toUpperCase(Locale.ROOT); } + private static void appendStarColumns( + Source source, boolean merged, List labels, List tables) { + if (!source.knownColumns()) { + return; + } + for (int i = 0; i < source.columns().size(); i++) { + String label = source.columns().get(i); + if (merged && containsIgnoreCase(labels, label)) { + continue; + } + labels.add(label); + tables.add(source.tables().get(i)); + } + } + + private static void dropStarColumns( + List columns, int from, List labels, List tables) { + for (int i = labels.size() - 1; i >= from; i--) { + if (containsIgnoreCase(columns, labels.get(i))) { + labels.remove(i); + tables.remove(i); + } + } + } + + private static boolean containsIgnoreCase(List values, String value) { + for (String candidate : values) { + if (candidate.equalsIgnoreCase(value)) { + return true; + } + } + return false; + } + private static List tokenize(String sql) { List tokens = new ArrayList<>(); int depth = 0; diff --git a/libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/LibsqlResultSetMetaDataTest.java b/libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/LibsqlResultSetMetaDataTest.java index b620833..6a1002f 100644 --- a/libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/LibsqlResultSetMetaDataTest.java +++ b/libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/LibsqlResultSetMetaDataTest.java @@ -477,6 +477,51 @@ void resolvesViewTableNameAndNullability() throws SQLException { assertThat(metadata.isNullable(2)).isEqualTo(ResultSetMetaData.columnNullable); } + @Test + void resolvesUsingJoinStarColumnsToTheLeftTable() throws SQLException { + LibsqlResultSetMetaData metadata = metadataForSql( + "SELECT * FROM a JOIN b USING(id) JOIN c", + schemaConnection(), + List.of(new Col("id", "INTEGER"), new Col("x", "TEXT"), new Col("y", "TEXT"), new Col("z", "TEXT"))); + + assertThat(metadata.getTableName(1)).isEqualTo("a"); + assertThat(metadata.getTableName(2)).isEqualTo("a"); + assertThat(metadata.getTableName(3)).isEqualTo("b"); + assertThat(metadata.getTableName(4)).isEqualTo("c"); + assertThat(metadata.isNullable(1)).isEqualTo(ResultSetMetaData.columnNoNulls); + } + + @Test + void resolvesGeneratedColumnStarColumnsThroughTableXinfo() throws SQLException { + LibsqlResultSetMetaData metadata = metadataForSql( + "SELECT * FROM g JOIN c ON 1", + schemaConnection(), + List.of( + new Col("p", "INTEGER"), + new Col("q", "INTEGER"), + new Col("r", "TEXT"), + new Col("z", "TEXT"))); + + assertThat(metadata.getTableName(1)).isEqualTo("g"); + assertThat(metadata.getTableName(2)).isEqualTo("g"); + assertThat(metadata.getTableName(3)).isEqualTo("g"); + assertThat(metadata.getTableName(4)).isEqualTo("c"); + assertThat(metadata.isNullable(3)).isEqualTo(ResultSetMetaData.columnNoNulls); + } + + @Test + void leavesEveryColumnUnknownWhenTheStarWidthDiffers() throws SQLException { + LibsqlResultSetMetaData metadata = metadataForSql( + "SELECT * FROM (VALUES(1)) v, c", + schemaConnection(), + List.of(new Col("column1", null), new Col("z", "TEXT"))); + + assertThat(metadata.getTableName(1)).isEmpty(); + assertThat(metadata.getTableName(2)).isEmpty(); + assertThat(metadata.isNullable(1)).isEqualTo(ResultSetMetaData.columnNullableUnknown); + assertThat(metadata.isNullable(2)).isEqualTo(ResultSetMetaData.columnNullableUnknown); + } + @Test void leavesUnsupportedShapesUnknown() throws SQLException { LibsqlResultSetMetaData expression = metadataForSql( @@ -516,10 +561,36 @@ private static LibsqlConnection schemaConnection() { "v", "CREATE VIEW v AS SELECT a, b FROM w", List.of(new FakeColumn("a", false, false), new FakeColumn("b", false, false)))); + tables.put( + "a", + new FakeTable( + "a", + "CREATE TABLE a (id INTEGER NOT NULL, x TEXT)", + List.of(new FakeColumn("id", true, false), new FakeColumn("x", false, false)))); + tables.put( + "b", + new FakeTable( + "b", + "CREATE TABLE b (id INTEGER, y TEXT)", + List.of(new FakeColumn("id", false, false), new FakeColumn("y", false, false)))); + tables.put( + "c", + new FakeTable( + "c", "CREATE TABLE c (z TEXT)", List.of(new FakeColumn("z", false, false)))); + tables.put( + "g", + new FakeTable( + "g", + "CREATE TABLE g (p INTEGER, q AS (p+1), r TEXT NOT NULL)", + List.of( + new FakeColumn("p", false, false), + new FakeColumn("q", false, false, true), + new FakeColumn("r", true, false)))); return new LibsqlConnection(null, null) { @Override public StmtResult executeStatement(Stmt stmt) throws SQLException { - if (!stmt.sql().contains("pragma_table_info")) { + boolean xinfo = stmt.sql().contains("pragma_table_xinfo"); + if (!xinfo && !stmt.sql().contains("pragma_table_info")) { return new StmtResult(List.of(), List.of(), 0, null); } String table = argument(stmt, 0); @@ -530,6 +601,9 @@ public StmtResult executeStatement(Stmt stmt) throws SQLException { } List rows = new ArrayList<>(); for (FakeColumn column : fake.columns()) { + if (column.generated() && !xinfo) { + continue; + } rows.add(new Row(List.of( Value.of(fake.name()), Value.of(fake.sql()), @@ -551,7 +625,11 @@ private static String argument(Stmt stmt, int index) { private record FakeTable(String name, String sql, List columns) {} - private record FakeColumn(String name, boolean notNull, boolean primaryKey) {} + private record FakeColumn(String name, boolean notNull, boolean primaryKey, boolean generated) { + FakeColumn(String name, boolean notNull, boolean primaryKey) { + this(name, notNull, primaryKey, false); + } + } private static LibsqlResultSetMetaData metadataForTable(String createSql, List cols) throws SQLException { List tableRows = cols.stream() diff --git a/libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/SqlTableReferenceTest.java b/libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/SqlTableReferenceTest.java index f7ded41..282d5b1 100644 --- a/libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/SqlTableReferenceTest.java +++ b/libsql-jdbc/src/test/java/com/github/magnusp/libsql/jdbc/SqlTableReferenceTest.java @@ -19,7 +19,17 @@ class SqlTableReferenceTest { "v", new SqlTableReference.TableDefinition("CREATE VIEW v AS SELECT a, b FROM w", List.of("a", "b")), "my table", - new SqlTableReference.TableDefinition("CREATE TABLE [my table] (c TEXT)", List.of("c"))); + new SqlTableReference.TableDefinition("CREATE TABLE [my table] (c TEXT)", List.of("c")), + "a", + new SqlTableReference.TableDefinition( + "CREATE TABLE a (id INTEGER NOT NULL, x TEXT)", List.of("id", "x")), + "b", + new SqlTableReference.TableDefinition("CREATE TABLE b (id INTEGER, y TEXT)", List.of("id", "y")), + "c", + new SqlTableReference.TableDefinition("CREATE TABLE c (z TEXT)", List.of("z")), + "g", + new SqlTableReference.TableDefinition( + "CREATE TABLE g (p INTEGER, q AS (p+1), r TEXT NOT NULL)", List.of("p", "q", "r"))); private static final SqlTableReference.Schema SCHEMA = reference -> { if (reference.table() == null) { @@ -97,6 +107,50 @@ void resolvesStarExpansion() throws SQLException { .containsExactly(new SqlTableReference.TableRef(null, "w"), new SqlTableReference.TableRef(null, "w")); } + @Test + void dropsTheDuplicateUsingColumnWhenExpandingStar() throws SQLException { + SqlTableReference.Resolution resolution = resolve("SELECT * FROM a JOIN b USING(id) JOIN c", 4); + + assertThat(resolution.labels()).containsExactly("id", "x", "y", "z"); + assertThat(resolution.tables()) + .containsExactly( + new SqlTableReference.TableRef(null, "a"), + new SqlTableReference.TableRef(null, "a"), + new SqlTableReference.TableRef(null, "b"), + new SqlTableReference.TableRef(null, "c")); + } + + @Test + void dropsTheDuplicateNaturalColumnWhenExpandingStar() throws SQLException { + SqlTableReference.Resolution resolution = resolve("SELECT * FROM a NATURAL JOIN b", 3); + + assertThat(resolution.labels()).containsExactly("id", "x", "y"); + assertThat(resolution.tables()) + .containsExactly( + new SqlTableReference.TableRef(null, "a"), + new SqlTableReference.TableRef(null, "a"), + new SqlTableReference.TableRef(null, "b")); + } + + @Test + void expandsGeneratedColumnsInStar() throws SQLException { + SqlTableReference.Resolution resolution = resolve("SELECT * FROM g JOIN c ON 1", 4); + + assertThat(resolution.labels()).containsExactly("p", "q", "r", "z"); + assertThat(resolution.tables()) + .containsExactly( + new SqlTableReference.TableRef(null, "g"), + new SqlTableReference.TableRef(null, "g"), + new SqlTableReference.TableRef(null, "g"), + new SqlTableReference.TableRef(null, "c")); + } + + @Test + void discardsTheResolutionWhenTheStarWidthDiffers() throws SQLException { + assertThat(resolve("SELECT * FROM (VALUES(1)) v, c", 2)).isNull(); + assertThat(resolve("SELECT * FROM unknown_source, c", 2)).isNull(); + } + @Test void resolvesSubqueryInFromClause() throws SQLException { SqlTableReference.Resolution resolution = resolve("SELECT q.a FROM (SELECT a FROM w) q"); @@ -187,4 +241,12 @@ void rejectsUnparseableInput() throws SQLException { private static SqlTableReference.Resolution resolve(String sql) throws SQLException { return SqlTableReference.resolve(sql, SCHEMA); } + + private static SqlTableReference.Resolution resolve(String sql, int columnCount) throws SQLException { + SqlTableReference.Resolution resolution = SqlTableReference.resolve(sql, SCHEMA, columnCount); + assertThat(resolution == null || resolution.labels().size() == columnCount) + .as("resolved column count equals the result column count") + .isTrue(); + return resolution; + } }