[
https://issues.apache.org/jira/browse/CALCITE-1150?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15280603#comment-15280603
]
Jinfeng Ni commented on CALCITE-1150:
-------------------------------------
[~julianhyde], I revised the patch based on your comments.
* Move SqlSelect.expanded into the validator, maybe in SelectScope. It is best
if validation does not mutate the parse tree. (Yes, I know expansion changes
the select list but don't want to make that hole of technical debt any deeper.)
** Removed the code of SqlSelect.expanded. Previously, this flag is needed to
avoid expanding select list multiple times. The new code will represent the
static star and dynamic star field differently. This makes that code
unnecessary. However, SqlValidatorImpl still has to expand select list, order
by exp, where/join condition, and group by exp. In some sense, the expand is
part of validation process : we validate and replace the original form with a
"validated" form. Validator's "originalExprs" is for the purpose of keeping
track of the "original" vs "expanded" expression.
* RelDataTypes are immutable, and your DynamicRecordType is mutable. I know you
need a "collector". I could accept using a mutable type just during validation,
then switch to an immutable dynamic record type on sql-to-rel. But if the
collector could be in a scope that would be even better.
** Per your suggestion, I use DynamicRecordType only in validation phase. In
RelOptTable.toRel(), when the rowType is dynamicStruct, we switch to an
immutable and regular record type.
* Do you really need "" to be a field in the record type? Is it not sufficient
for "" to be a field reference?
** I'm not sure if I understand the question. But I choose to use "**" as the
dynamic star prefix, to differentiate from static star "". Also, introduce a
new SqlTypeName for dynamic star field. This new SqlTypeName is useful to
identify the dynamic star fields in the rowType. When the physical plan is
built, we rely on such knowledge to prepare the dynamic expansion in execution
time.
* How do you plan to represent the row type of SELECT * FROM (SELECT * FROM
t1), (SELECT * FROM t2))? There are unresolved stars on both sides.
** I added one unit test for such case. Basically, the top project will have
two dynamic star fields. One is named as "**", the other one is named as "**0",
due to naming conflict. The * column in outer query block is actually treated
as a regular star column; it's expanded into two dynamic star fields in
validation phase. It will also resolve correctly for SELECT * FROM (SELECT *,
col1 FROM t1), (SELECT *, col2 FROM t2)).
* Given RelDataType t, I'd prefer t.isDynamicStruct() to t instanceof
DynamicRecordType.
** Done. Add isDynamicStruct() to interface RelDataType.
* Minor nit-pick: use new ArrayList<>() rather than Lists.newArrayList() for
new code.
** Done.
* Add a sql-to-rel test SELECT * FROM nonDynamicTable WHERE EXISTS (SELECT *
FROM dynamicTable). The select clause of an EXISTS sub-query should always be
ignored.
** Done. The select clause will be ignored, although the scan will return all
the columns. That's the same behavior for the case where static table is used
in the exist sub-query.
I also modify couple of expected error messages in SqlValidatorTest. According
to the code, the error message should use the original expression, in stead of
expanded expression.
> Create the a new DynamicRecordType, avoiding star expansion when working with
> this type
> ---------------------------------------------------------------------------------------
>
> Key: CALCITE-1150
> URL: https://issues.apache.org/jira/browse/CALCITE-1150
> Project: Calcite
> Issue Type: Improvement
> Reporter: Jacques Nadeau
> Assignee: Julian Hyde
>
> DynamicRecordType can be used to leverage user-provided field implications to
> avoid schema analysis until execution.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
