github-actions[bot] commented on code in PR #64395:
URL: https://github.com/apache/doris/pull/64395#discussion_r3400421996
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/CopyFromDesc.java:
##########
@@ -203,23 +202,30 @@ private boolean getFileColumnNames(boolean addDeleteSign)
throws AnalysisExcepti
if (exprList == null) {
return false;
}
- List<SlotRef> slotRefs = Lists.newArrayList();
- // Expr.collectList(exprList, SlotRef.class, slotRefs);
+ boolean hasFileColumnPlaceholder = false;
Set<String> columnSet = Sets.newTreeSet(String.CASE_INSENSITIVE_ORDER);
- for (SlotRef slotRef : slotRefs) {
- String columnName = slotRef.getColumnName();
- if (columnName.startsWith(DOLLAR)) {
- if (fileColumns.size() > 0) {
+ for (Expression expr : exprList) {
+ for (UnboundSlot slot :
expr.<UnboundSlot>collectToList(UnboundSlot.class::isInstance)) {
+ String columnName = slot.getName();
+ if (columnName.startsWith(DOLLAR)) {
+ if (!fileColumns.isEmpty()) {
+ throw new AnalysisException("can not mix column name
and dollar sign");
+ }
+ hasFileColumnPlaceholder = true;
+ continue;
+ }
+ if (hasFileColumnPlaceholder) {
throw new AnalysisException("can not mix column name and
dollar sign");
}
- return false;
- }
- if (columnSet.add(columnName)) {
- fileColumns.add(columnName);
+ if (columnSet.add(columnName)) {
+ fileColumns.add(columnName);
+ }
}
}
+ if (hasFileColumnPlaceholder) {
Review Comment:
This still misses placeholders that appear only in the COPY file filter. For
example, `COPY INTO t(c) FROM (SELECT 1 FROM @s('f') WHERE $1 = 'x') ...`
reaches this method with no `UnboundSlot` in `exprList`, so
`hasFileColumnPlaceholder` stays false and the method returns `true` with
`fileColumns` still empty. `CopyIntoFileSlots` then adds no `$1` scope slot,
and translating `fileFilterExpr` still fails with unknown column `$1`. This is
distinct from the earlier `SELECT $1` thread: the current patch fixes
placeholders in the select list, but not placeholders used only by the filter.
Please include `fileFilterExpr` when deciding/generating placeholder columns,
or derive file slots from both mappings and filters, and add a regression for
this filter-only `$N` case.
##########
regression-test/suites/load_p0/copy_into/test_copy_into.groovy:
##########
@@ -150,6 +150,47 @@ suite("test_copy_into", "p0") {
}
assertTrue(false, "should not come here")
}
+
+ def csvStageName = "test_copy_into_csv"
+ try_sql """drop stage if exists ${csvStageName}"""
+ sql """
+ create stage if not exists ${csvStageName}
+ properties ('endpoint' = '${getS3Endpoint()}' ,
+ 'region' = '${getS3Region()}' ,
+ 'bucket' = '${getS3BucketName()}' ,
+ 'prefix' = 'regression' ,
+ 'ak' = '${getS3AK()}' ,
+ 'sk' = '${getS3SK()}' ,
+ 'provider' = '${getS3Provider()}',
+ 'access_type' = 'aksk',
+ 'default.file.column_separator' = "|");
+ """
+
+ sql """ DROP TABLE IF EXISTS copy_into_select_placeholder; """
+ sql """
+ CREATE TABLE copy_into_select_placeholder (
+ p_partkey int NOT NULL DEFAULT "1",
+ p_name VARCHAR(55) NOT NULL DEFAULT "2",
+ p_mfgr VARCHAR(25) NOT NULL DEFAULT "3"
+ )ENGINE=OLAP
+ DUPLICATE KEY(`p_partkey`)
+ COMMENT "OLAP"
+ DISTRIBUTED BY HASH(`p_partkey`) BUCKETS 3;
+ """
+
+ result = sql """
+ copy into copy_into_select_placeholder
+ from (select \$1, \$2, \$3 from
@${csvStageName}('tpch/sf1/part.csv.split00.gz'))
+ properties ('file.type' = 'csv', 'file.column_separator' = '|',
+ 'file.compression' = 'gz', 'copy.async' = 'false');
+ """
+ logger.info("copy select placeholder result: " + result)
+ assertTrue(result.size() == 1)
+ assertTrue(result[0][1].equals("FINISHED"),
+ "Finish copy into, state=" + result[0][1] + ", expected
state=FINISHED")
+ order_qt_select_placeholder_count " SELECT COUNT(*) > 0 FROM
copy_into_select_placeholder; "
Review Comment:
This adds the first `order_qt` tag to `test_copy_into.groovy`, but the PR
does not add `regression-test/data/load_p0/copy_into/test_copy_into.out`. In
normal comparison mode, `quickRunTest` checks `context.outputFile.exists()` and
throws `Missing outputFile` before it can execute this query, so the regression
fails independently of the COPY INTO fix. Please generate and commit the `.out`
block for `select_placeholder_count`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
