Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20040 )

Change subject: IMPALA-12200: Cap stats NDV from SetOperationStmt.createMetadata
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/20040/3/fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java
File fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java:

http://gerrit.cloudera.org:8080/#/c/20040/3/fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java@185
PS3, Line 185:   public Column getSourceColumn() {
> It would be better to have this function return the estimated cardinality s
The intention to return Column here is so that duplicate Column reference from 
different Expr/SlotRef can be eliminated through the use of HashSet in 
SetOperationStmt.createMetadata(). We can not just return an integer from here 
since the commonalities then can not be inferred.


http://gerrit.cloudera.org:8080/#/c/20040/3/fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java@195
PS3, Line 195:         if (slotRef == null || !slotRef.hasDesc() || 
slotRef.getDesc() == this) {
> Checks seem redundant. Are there DCHECKs somewhere that ensure these are co
sourceExprs_ is a List<Expr>, and an Expr is the very base class of all kind of 
expression in Impala.
First and second predicate is needed because not all member of sourceExprs_ is 
a SlotRef expr, and even a SlotRef expr may not have SlotDescriptor.

The third predicate is a defensive one, since there is no DCHECK/Precondition 
anywhere that prevent addSourceExpr(Expr expr) to accept expr with equal 
SlotDescriptor as 'this'.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0bb2eff5005fdfb11adf31499214c63dd552c05
Gerrit-Change-Number: 20040
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Kurt Deschler <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Reviewer: Wenzhe Zhou <[email protected]>
Gerrit-Comment-Date: Tue, 13 Jun 2023 16:46:36 +0000
Gerrit-HasComments: Yes

Reply via email to