[ 
https://issues.apache.org/jira/browse/DRILL-5920?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16234465#comment-16234465
 ] 

Paul Rogers commented on DRILL-5920:
------------------------------------

Detailed analysis.

Working back through the code, the mystery deepens. 

{code}
  public static ProjectPushInfo getColumns(RelDataType rowType, List<RexNode> 
projects) {
    final List<String> fieldNames = rowType.getFieldNames();
{code}

Where:

{code}
fieldNames = [*, col1, columns]
{code}

Which, frankly, makes no sense. How can we have all columns (“*”), a physical 
column (“columns”) and an alias for that column “col1” in the same name space? 
Isn’t it true that, in SQL, an alias hides the original name?

Further back, the RelDataTypeHolder, holds a list of RelDataTypeFieldImpl 
objects as:

{code}
[#0: * ANY, #1: col1 ANY, #2: columns ANY]
{code}

There is no data in each object to differentiate the different categories of 
fields. The …FieldImpl class seems to be a Calcite class, so this is about as 
far as I got.

Taking a step back, it seems clear that any table can support the wildcard. 
There is a clear difference between base table columns and aliases. We may know 
nothing about the referenced table columns, but we can tell, from syntax, which 
name is an alias and which is a table column.

Working way back, here is the text of the {{SqlNode}} in {{getQueryPlan()}}:

{code}
SELECT MAX(`textinput/input1.csv`.`columns`[1]) AS `col1`
FROM `cp`.`textinput/input1.csv` AS `textinput/input1.csv`
WHERE `textinput/input1.csv`.`col1` IS NOT NULL
{code}

So, the parser understood the query correctly.

Looking at other bits, on this line:

{code}
    final ConvertedRelNode convertedRelNode = validateAndConvert(sqlNode);
{code}

The {{EasyGroupScan}} has:

{code}
EasyGroupScan [selectionRoot=classpath:/textinput/input1.csv, numFiles=1, 
columns=[`*`], files=[classpath:/textinput/input1.csv]]
{code}

I suppose this means that the table does not know it’s columns? So we provide 
the wildcard instead?

The field list seems to be built up here:

{code}
public class RelDataTypeHolder {
  public RelDataTypeField getField(RelDataTypeFactory typeFactory, String 
fieldName) {
{code}

First we get “*”, then “col1” (many times), and finally “columns” (six times). 
Each time we squirrel alway the name in our field list, resulting in the bogus 
list shown earlier. Here, I wonder:

* Why do we save the wildcard? That tells us nothing.
* Why is “col1”, which is an alias, resolved against the base table?

Let’s look at “col1". This comes to us via;

{code}
public class SqlValidatorUtil {
  public RelDataTypeField field(RelDataType rowType, String alias) {
    return SqlValidatorUtil.lookupField(caseSensitive, elideRecord, rowType,
        alias);
  }
{code}

Where alias = “col1”

The above calls this:

{code}
public abstract class ListScope extends DelegatingScope {
...
  public Pair<String, SqlValidatorNamespace>
  findQualifyingTableName(final String columnName, SqlNode ctx) {
    int count = 0;
    Pair<String, SqlValidatorNamespace> tableName = null;
    for (Pair<String, SqlValidatorNamespace> child : children) {
      final RelDataType rowType = child.right.getRowType();
      if (validator.catalogReader.field(rowType, columnName) != null) {
        tableName = child;
        count++;
      }
    }
{code}

Which has this namespace:

{code}
[<textinput/input1.csv, 
org.apache.calcite.sql.validate.IdentifierNamespace@328a728f>]
{code}

Because we always resolve the column (making up columns as needed), we never 
return null.

All of this is deep in a stack of DrillValidator sql converters where I’m 
getting a bit lost.

Shouldn’t names first be resolved in the SELECT statement’s name space of 
aliases, and only then in the table’s name space? Otherwise, if we go the other 
way around (table first, then local name space), we’ll always resolve to 
columns, even when the name is an alias, resulting in the original bug I’m 
facing.

> Drill incorrectly projects column aliases to scan operator
> ----------------------------------------------------------
>
>                 Key: DRILL-5920
>                 URL: https://issues.apache.org/jira/browse/DRILL-5920
>             Project: Apache Drill
>          Issue Type: Bug
>    Affects Versions: 1.10.0
>            Reporter: Paul Rogers
>            Priority: Major
>
> The {{TestNewTextReader.ensureColumnNameDisplayedinError}} unit test runs 
> this query:
> {code}
> select max(columns[1]) as col1
> from cp.`textinput/input1.csv`
> where col1 is not null
> {code}
> The following appears in the {{SubScan}} for the {{TextFormatPlugin}}:
> {noformat}
> [`col1`, `columns`[1]]
> {noformat}
> This is clearly wrong. The actual table column is {{columns}} (and, 
> specifically, element 1.) {{col1} is an alias that should never have been 
> pushed down to the data source because the data source does not know about 
> aliases.
> Further, the projection list makes no distinction between the "real" and 
> "alias" columns, so, to the data source, both look like real table columns.
> The current workaround is to create a nullable int column for {{col1}} which 
> is, presumably, replaced by a later projection operator.
> Because this behavior is wrong, we must think though all the possible failure 
> cases and how to handle them in this incorrect design. What if the alias 
> matches an (expensive) table column? What if the alias is the same as some 
> base column in the same query?
> {code}
> SELECT a as b, b as c FROM ...
> {code}
> Incorrect name handling may work in many cases, but it does lead to problems 
> because the behavior is not following the accepted SQL standards.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to