Aman Sinha has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/22052 )

Change subject: IMPALA-13540: Calcite planner: fix wrong results for set 
operators
......................................................................


Patch Set 4:

(3 comments)

> Patch Set 2:
>
> (1 comment)

http://gerrit.cloudera.org:8080/#/c/22052/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/22052/4//COMMIT_MSG@17
PS4, Line 17: result
nit: I meant use 'row' here as well.  Sorry if this wasn't clear in my previous 
comment.


http://gerrit.cloudera.org:8080/#/c/22052/4/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaCustomOperatorTable.java
File 
java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaCustomOperatorTable.java:

http://gerrit.cloudera.org:8080/#/c/22052/4/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaCustomOperatorTable.java@228
PS4, Line 228: Changed all the set operators.
'Changed' doesn't sound right.  Did you mean overload or override ?  Basically 
you are saying this overrides the default Calcite behavior.


http://gerrit.cloudera.org:8080/#/c/22052/4/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaCustomOperatorTable.java@230
PS4, Line 230: The SQL standard agrees with Calcite,
nit: Let's say Calcite follows the SQL standard (otherwise it makes it sound 
like the SQL standard came later than Calcite's implementation).

About Oracle,  note the following statement in their docs (Oracle 19 version):
https://docs.oracle.com/en/database/oracle/oracle-database/19/sqlrf/The-UNION-ALL-INTERSECT-MINUS-Operators.html
"Note:To comply with emerging SQL standards, a future release of Oracle will 
give the INTERSECT operator greater precedence than the other set operators. 
Therefore, you should use parentheses to specify order of evaluation in queries 
that use the INTERSECT operator with other set operators."

We could just remove the reference to Oracle and Hive here since there are 
other DBMSs that might have the same behavior. Here we are only comparing 
Calcite and impala.



--
To view, visit http://gerrit.cloudera.org:8080/22052
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic52661a30cc90534ea1a20868799edf9ceed13b6
Gerrit-Change-Number: 22052
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <[email protected]>
Gerrit-Reviewer: Aman Sinha <[email protected]>
Gerrit-Reviewer: Anonymous Coward (816)
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Steve Carlin <[email protected]>
Gerrit-Comment-Date: Wed, 20 Nov 2024 01:22:51 +0000
Gerrit-HasComments: Yes

Reply via email to