Copilot commented on code in PR #39590:
URL: https://github.com/apache/shardingsphere/pull/39590#discussion_r3840121962
##########
fix_bitor.py:
##########
@@ -0,0 +1,47 @@
+import os
Review Comment:
This newly added Python source file has no ASF license header. The
repository runs Apache RAT and its maintained Python utilities use the standard
header, so this file can fail the required license check; add the header or
remove this temporary script.
##########
patch_doris_and_tests.py:
##########
@@ -0,0 +1,127 @@
+import os
+
+# 1. Patch DALStatement.g4
+dal_file =
'parser/sql/engine/dialect/doris/src/main/antlr4/imports/doris/DALStatement.g4'
Review Comment:
None of the four newly added root-level Python patch scripts has an ASF
license header, while `src/resources/rat.txt` does not exclude `*.py` and
existing repository Python files carry the header. The claimed `-Pcheck` build
will therefore fail the RAT license check. Add headers to every intentionally
maintained script or, preferably for these unreferenced one-off mutation tools,
remove them from the pull request.
##########
fix_all.py:
##########
@@ -0,0 +1,102 @@
+import re
Review Comment:
This newly added Python source file has no ASF license header. The
repository runs Apache RAT and its maintained Python utilities use the standard
header, so this file can fail the required license check; add the header or
remove this temporary script.
This issue also appears on line 3 of the same file.
##########
parser/sql/engine/dialect/doris/src/main/antlr4/imports/doris/DALStatement.g4:
##########
@@ -472,7 +472,7 @@ adminSetReplicaVersion
;
adminCopyTablet
- : ADMIN COPY TABLET NUMBER_ propertiesClause?
+ : ADMIN COPY TABLET (NUMBER_ | identifier) propertiesClause?
Review Comment:
This unrelated change broadens `ADMIN COPY TABLET` from a numeric tablet ID
to any identifier, so malformed nonnumeric tablet IDs become accepted by the
Doris parser without a corresponding requirement or test. Please keep this
BITOR/BITAND PR focused and revert this grammar change; a DAL syntax change
should be validated separately.
This issue also appears on line 567 of the same file.
##########
parser/sql/engine/dialect/doris/src/main/java/org/apache/shardingsphere/sql/parser/engine/doris/visitor/statement/type/DorisDMLStatementVisitor.java:
##########
@@ -647,4 +647,5 @@ public ASTNode visitWindowingClause(final
WindowingClauseContext ctx) {
}
return result;
}
+
Review Comment:
This adds a meaningless blank line immediately before the class terminator
and is the only change in this Java file. Please remove it so the PR does not
carry an unrelated formatting-only delta.
##########
parser/sql/engine/dialect/doris/src/main/antlr4/imports/doris/BaseRule.g4:
##########
@@ -1105,6 +1105,8 @@ aggregationFunctionName
// DORIS ADDED BEGIN
bitwiseBinaryFunctionName
: BITXOR
+ | BITOR
+ | BITAND
Review Comment:
These alternatives reuse `bitwiseFunction`, whose visitor at
`DorisStatementVisitor.java:1193-1197` wraps every `expr` as a
`LiteralExpressionSegment` instead of visiting it. Consequently, `BITOR(col_a,
col_b)` and nested expressions lose their column/expression AST semantics even
though the grammar accepts full expressions. Update the shared visitor to visit
each argument and add a non-literal regression case.
This issue also appears on line 1108 of the same file.
##########
patch_visitor.py:
##########
@@ -0,0 +1,51 @@
+import os
Review Comment:
This newly added Python source file has no ASF license header. The
repository runs Apache RAT and its maintained Python utilities use the standard
header, so this file can fail the required license check; add the header or
remove this temporary script.
##########
patch_doris_and_tests.py:
##########
@@ -0,0 +1,127 @@
+import os
+
+# 1. Patch DALStatement.g4
+dal_file =
'parser/sql/engine/dialect/doris/src/main/antlr4/imports/doris/DALStatement.g4'
Review Comment:
This is an ad-hoc patcher that rewrites `DALStatement.g4` and parser
fixtures unrelated to BITOR/BITAND. Nothing in the repository invokes it, and
the new Python file has no ASF header even though the repository RAT check runs
during `verify`; please remove this temporary script instead of committing an
intermediate repair step.
##########
remove_bitnot.py:
##########
@@ -0,0 +1,92 @@
+import re
+
+# 1. Remove BITNOT from DorisKeyword.g4
+keyword_file =
'parser/sql/engine/dialect/doris/src/main/antlr4/imports/doris/DorisKeyword.g4'
Review Comment:
This is a destructive rollback helper that deletes grammar and test content
and searches for a visitor file from whatever directory it is run in. It is not
part of the build, has no ASF header, and represents an intermediate BITNOT
experiment rather than the requested change; please remove it from the PR.
##########
remove_bitnot.py:
##########
@@ -0,0 +1,92 @@
+import re
Review Comment:
This newly added Python source file has no ASF license header. The
repository runs Apache RAT and its maintained Python utilities use the standard
header, so this file can fail the required license check; add the header or
remove this temporary script.
##########
patch_visitor.py:
##########
@@ -0,0 +1,51 @@
+import os
+
+# Find file dynamically
+target_file = "DorisDMLStatementVisitor.java"
+file_path = None
+
+for root, dirs, files in os.walk("."):
+ if target_file in files:
+ file_path = os.path.join(root, target_file)
+ break
+
+if not file_path:
+ print(f"❌ Error: Could not find {target_file} in current directory.")
+ exit(1)
+
+print(f"Found target file at: {file_path}")
+
+with open(file_path, 'r') as f:
+ content = f.read()
+
+# 1. Add import for BitwiseNotFunctionContext if needed
+if 'BitwiseNotFunctionContext' not in content:
+ content = content.replace(
+ 'import
org.apache.shardingsphere.sql.parser.autogen.DorisStatementParser.BitwiseFunctionContext;',
+ 'import
org.apache.shardingsphere.sql.parser.autogen.DorisStatementParser.BitwiseFunctionContext;\nimport
org.apache.shardingsphere.sql.parser.autogen.DorisStatementParser.BitwiseNotFunctionContext;'
Review Comment:
This script targets `DorisDMLStatementVisitor.java`, but the
`BitwiseFunctionContext` import and `visitSpecialFunction` dispatch are owned
by `DorisStatementVisitor.java`; the replacement strings therefore do nothing
for the target. It then appends a method referring to an unimported
`BitwiseNotFunctionContext`, so running the script produces uncompilable Java.
Remove this intermediate script or update it to target the actual owner and
verify the result.
--
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]
