From 75033d85d939450b13792b929aa4ddcece18422c Mon Sep 17 00:00:00 2001 From: Tyagiquamar <347892697+Tyagiquamar@users.noreply.github.com> Date: Sat, 24 Oct 2026 09:41:53 +0530 Subject: [PATCH 1/2] test(copy): add reproducer for COPY FROM subquery reusing a variable Adds issue1365.test, which copies node and relationship tables from subqueries whose RETURN list reuses one variable. Expected values follow the positional mapping of result columns onto the table schema. Signed-off-by: Tyagiquamar <347892697+Tyagiquamar@users.noreply.github.com> --- test/test_files/issue/issue1365.test | 100 +++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 test/test_files/issue/issue1365.test diff --git a/test/test_files/issue/issue1365.test b/test/test_files/issue/issue1365.test new file mode 100644 index 0000000000..1d51fbd7af --- /dev/null +++ b/test/test_files/issue/issue1365.test @@ -0,0 +1,100 @@ +-DATASET CSV empty + +-- + +-CASE Issue1365CopyFromSubqueryDuplicateVariable +# Reproduction for github issue #1365: +# COPY FROM (subquery) whose RETURN reuses a variable (RETURN i, i, i) +# failed with a spurious NULL primary key, and a reused variable in a non-key +# position was inserted as NULL instead of its value. +# Root cause: Binder::bindQueryScanSource renamed the subquery result columns by +# calling setAlias on the shared Expression objects. A variable bound twice is +# one object, so it kept only the last expected name, and the copy binder - which +# matched table properties to columns through that name - fell back to the column +# default. Result columns are now mapped positionally against the expected column +# names, so no bound expression is mutated. + +-STATEMENT CREATE NODE TABLE q3(ID INT64 PRIMARY KEY, a INT64, b INT64); +---- ok + +-STATEMENT COPY q3 FROM (UNWIND range(20, 22) AS i RETURN i, i, i); +---- ok + +-STATEMENT MATCH (n:q3) RETURN n.ID, n.a, n.b ORDER BY n.ID; +---- 3 +20|20|20 +21|21|21 +22|22|22 + +# The same failure with a string column, reported while testing PR #1350. +-STATEMENT CREATE NODE TABLE artist(ID INT64, name STRING, rank INT64, PRIMARY KEY(ID)); +---- ok + +-STATEMENT COPY artist FROM (UNWIND range(20, 22) AS i RETURN i, 'Grace', i); +---- ok + +-STATEMENT MATCH (n:artist) RETURN n.ID, n.name, n.rank ORDER BY n.ID; +---- 3 +20|Grace|20 +21|Grace|21 +22|Grace|22 + +# The workarounds from the issue, where every RETURN expression is a distinct +# object, already worked and must keep working. +-STATEMENT COPY q3 FROM (UNWIND range(30, 31) AS i RETURN i, i + 0, i + 1); +---- ok + +-STATEMENT MATCH (n:q3) RETURN n.ID, n.a, n.b ORDER BY n.ID; +---- 5 +20|20|20 +21|21|21 +22|22|22 +30|30|31 +31|31|32 + +# An aliased RETURN list keeps mapping positionally onto the table schema. +-STATEMENT COPY q3 FROM (UNWIND range(40, 40) AS i RETURN i AS x, i + 1 AS y, i + 2 AS z); +---- ok + +-STATEMENT MATCH (n:q3) RETURN n.ID, n.a, n.b ORDER BY n.ID; +---- 6 +20|20|20 +21|21|21 +22|22|22 +30|30|31 +31|31|32 +40|41|42 + +# A column list assigns the source positions to properties, so the property +# order is not the source order. Before the fix the reused variable kept only +# the last expected name and the other property took its default (NULL). +-STATEMENT CREATE NODE TABLE r2(ID INT64 PRIMARY KEY, a INT64); +---- ok + +-STATEMENT COPY r2 (a, ID) FROM (UNWIND range(1, 2) AS i RETURN i, i); +---- ok + +-STATEMENT MATCH (n:r2) RETURN n.ID, n.a ORDER BY n.ID; +---- 2 +1|1 +2|2 + +# Relationship copy: source and destination are taken by position, the +# remaining properties by name. +-STATEMENT CREATE NODE TABLE person(ID INT64 PRIMARY KEY); +---- ok + +-STATEMENT CREATE REL TABLE knows(FROM person TO person, w INT64, k INT64); +---- ok + +-STATEMENT COPY person FROM (UNWIND range(1, 3) AS i RETURN i); +---- ok + +-STATEMENT COPY knows FROM (UNWIND range(1, 3) AS i RETURN i, i, i, i); +---- ok + +-STATEMENT MATCH (a:person)-[e:knows]->(b:person) RETURN e.w, e.k ORDER BY e.w, e.k; +---- 3 +1|1 +2|2 +3|3 From 890feb0694eaf136e1b9cb356c9ba62581f1ebd8 Mon Sep 17 00:00:00 2001 From: Tyagiquamar <347892697+Tyagiquamar@users.noreply.github.com> Date: Sat, 24 Oct 2026 09:44:52 +0530 Subject: [PATCH 2/2] fix(binder): map COPY FROM subquery columns by position Binder::bindQueryScanSource renamed the subquery result columns in place with setAlias. A RETURN list that binds the same variable twice resolves both positions to one shared Expression object, so the object kept only the last expected name and the copy binder, which looked columns up by that name, fell back to the column default. A reused primary key column then raised a NULL PK violation and a reused non-key column was inserted as NULL. Scan sources name their leading columns from the expected schema by position (file, object and parameter readers do this while validating the count, and bindQueryScanSource validates the count too), so bind each table property against that position and stop mutating the bound subquery expressions. Columns beyond the expected names are copy warnings and a source bound without expected names keeps its own, so both still resolve by name. Signed-off-by: Tyagiquamar <347892697+Tyagiquamar@users.noreply.github.com> --- src/binder/bind/bind_file_scan.cpp | 6 ++--- src/binder/bind/copy/bind_copy_from.cpp | 36 +++++++++++++++---------- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/src/binder/bind/bind_file_scan.cpp b/src/binder/bind/bind_file_scan.cpp index 8297f1f5c2..e9a533a09c 100644 --- a/src/binder/bind/bind_file_scan.cpp +++ b/src/binder/bind/bind_file_scan.cpp @@ -198,9 +198,9 @@ std::unique_ptr Binder::bindQueryScanSource(const BaseScanS throw BinderException(std::format("Query returns {} columns but {} columns were expected.", columns.size(), columnNames.size())); } - for (auto i = 0u; i < columns.size(); ++i) { - columns[i]->setAlias(columnNames[i]); - } + // The columns are matched to columnNames by position when the COPY operator is bound. They + // must not be renamed here: a RETURN list can bind the same Expression object more than once + // (e.g. RETURN i, i), and setAlias would leave that shared object holding one name only. auto scanInfo = BoundQueryScanSourceInfo(bindParsingOptions(options)); return std::make_unique(std::move(boundStatement), std::move(scanInfo)); } diff --git a/src/binder/bind/copy/bind_copy_from.cpp b/src/binder/bind/copy/bind_copy_from.cpp index a2715cabc8..fa4063fe6a 100644 --- a/src/binder/bind/copy/bind_copy_from.cpp +++ b/src/binder/bind/copy/bind_copy_from.cpp @@ -116,18 +116,26 @@ static void bindExpectedRelColumns(const RelGroupCatalogEntry& entry, const CopyFromColumnInfo& info, std::vector& columnNames, std::vector& columnTypes); +// Returns the source column that feeds a table property. A scan source names its leading columns +// from expectedColumnNames by position, so the position decides which column a property reads. +// The name of the bound expression cannot be used for this: a query source may return one shared +// Expression object for several columns (e.g. RETURN i, i). Columns past the expected ones carry +// copy warnings. A source bound without expected names, such as a table whose only column is +// SERIAL, keeps the names it read from the file and is matched by those names. static std::pair> matchColumnExpression( - const expression_vector& columns, const PropertyDefinition& property, - ExpressionBinder& expressionBinder) { - for (auto& column : columns) { - if (property.getName() == column->toString()) { - if (column->dataType == property.getType()) { - return {ColumnEvaluateType::REFERENCE, column}; - } else { - return {ColumnEvaluateType::CAST, - expressionBinder.forceCast(column, property.getType())}; - } + const expression_vector& columns, const std::vector& expectedColumnNames, + const PropertyDefinition& property, ExpressionBinder& expressionBinder) { + for (auto i = 0u; i < columns.size(); ++i) { + auto columnName = + i < expectedColumnNames.size() ? expectedColumnNames[i] : columns[i]->toString(); + if (property.getName() != columnName) { + continue; + } + auto& column = columns[i]; + if (column->dataType == property.getType()) { + return {ColumnEvaluateType::REFERENCE, column}; } + return {ColumnEvaluateType::CAST, expressionBinder.forceCast(column, property.getType())}; } return {ColumnEvaluateType::DEFAULT, expressionBinder.bindExpression(*property.defaultExpr)}; } @@ -163,8 +171,8 @@ BoundCopyFromInfo Binder::bindCopyNodeFromInfo(std::string tableName, expression_vector columns; std::vector evaluateTypes; for (auto& property : properties) { - auto [evaluateType, column] = - matchColumnExpression(boundSource->getColumns(), property, expressionBinder); + auto [evaluateType, column] = matchColumnExpression(boundSource->getColumns(), + expectedColumnNames, property, expressionBinder); columns.push_back(column); evaluateTypes.push_back(evaluateType); } @@ -247,8 +255,8 @@ BoundCopyFromInfo Binder::bindCopyRelFromInfo(std::string tableName, ColumnEvaluateType::REFERENCE, ColumnEvaluateType::REFERENCE}; for (auto i = 1u; i < properties.size(); ++i) { // skip internal ID auto& property = properties[i]; - auto [evaluateType, column] = - matchColumnExpression(boundSource->getColumns(), property, expressionBinder); + auto [evaluateType, column] = matchColumnExpression(boundSource->getColumns(), + expectedColumnNames, property, expressionBinder); columnExprs.push_back(column); evaluateTypes.push_back(evaluateType); }