impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Kathy Sun (Code Review)" <>
Subject [Impala-ASF-CR] Infomation schema (preview for frontend)
Date Tue, 23 Aug 2016 19:13:47 GMT
Kathy Sun has posted comments on this change.

Change subject: Infomation_schema (preview for frontend)

Patch Set 8:


Please skip patch 7, it's a wrong reformat. 
Patch 8 is a new reformat using .clang_format

below is the reply I was working on, those will take effect in next patch.
File be/src/exec/

PS6, Line 100: alue = m
> Constants should be named like: COL_NAMES

PS6, Line 136: xt** ctxs 
> I think we can get rid of the 'tuple_pool' variable and just use row_batch-

Line 153: 
> This loop looks sufficient to me to materialise any number of rows.
unfortunately... I think it's not. I'm still testing this. discuss offline.

Line 168: 
> Our coding standard says to use '++idx'. Not a big deal but I think it's ea

Line 192
> You can actually remove DebugString(). We don't have it for all scan nodes.
nice, that's a relief. :P
File fe/src/main/java/com/cloudera/impala/analysis/

Line 532:         case VIEW_METADATA:
> Can you add a TODO like:

Line 533:         case SELECT:
> There are some tabs instead of spaces here. Let's just figure out how to se
File fe/src/main/java/com/cloudera/impala/catalog/

PS6, Line 34: InformationSchemaTable
> I spent some time thinking about this and I'm thinking we should call this 
to discuss in 1o1

Line 45: 
> Your text editor is leaving in trailing whitespace - it should be possible 

PS6, Line 52: Name
> We should keep the column names lower-case (they're converted to lower-case

Line 89:   /**
> Is this ever called? I think since isLoaded() is always true, this should n

Line 109:     TResultSetMetadata resultSchema = new TResultSetMetadata();
> Comment not needed.
File fe/src/main/java/com/cloudera/impala/planner/

PS6, Line 42: InfoSchemaScanNode
> Maybe this should be SystemTableScanNode (assuming we do the rename I sugge
discuss offline

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I7adbeb45220c468e43b424d70c30b952f6cec2cd
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Kathy Sun <>
Gerrit-Reviewer: Kathy Sun <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-HasComments: Yes

View raw message