Skip to content

Fix VirtualColumn related issues in window expressions#15119

Merged
abhishekagarwal87 merged 117 commits into
apache:masterfrom
kgyrtkirk:windowing-fixes-string-min-max
Oct 23, 2023
Merged

Fix VirtualColumn related issues in window expressions#15119
abhishekagarwal87 merged 117 commits into
apache:masterfrom
kgyrtkirk:windowing-fixes-string-min-max

Conversation

@kgyrtkirk

Copy link
Copy Markdown
Member

for some exotic queries like:

  SELECT
  	'_'||dim1,
    MIN(cast(0 as double)) OVER (),
    MIN(cast((cnt||cnt) as bigint)) OVER ()
  FROM foo

the compilation have resulted in NPE -s mostly because VirtualColumn -s were not handled properly

Comment thread sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidQuery.java Outdated
throw DruidException.defensive("Column[%s] not found!", column);
}
retVal = racColumn.toAccessor();
retVal = racColumn == null ? null : racColumn.toAccessor();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading this code has made me realize that we need to do a containsKey() check in order to actually take advantage of the cache. I.e. if we cache that it was null, we are going to keep asking and overwriting the cache for that column because we are checking the result of .get() instead of .containsKey().

I'm leaving this comment so that we have a shared understanding, not because I'm asking it to be adjusted in this PR.

@abhishekagarwal87
abhishekagarwal87 merged commit b95035f into apache:master Oct 23, 2023
kgyrtkirk added a commit to kgyrtkirk/druid that referenced this pull request Oct 23, 2023
for some exotic queries like:

  SELECT
  	'_'||dim1,
    MIN(cast(0 as double)) OVER (),
    MIN(cast((cnt||cnt) as bigint)) OVER ()
  FROM foo
the compilation have resulted in NPE -s mostly because VirtualColumn -s were not handled properly

(cherry picked from commit b95035f)
abhishekagarwal87 pushed a commit that referenced this pull request Oct 23, 2023
for some exotic queries like:

  SELECT
  	'_'||dim1,
    MIN(cast(0 as double)) OVER (),
    MIN(cast((cnt||cnt) as bigint)) OVER ()
  FROM foo
the compilation have resulted in NPE -s mostly because VirtualColumn -s were not handled properly

(cherry picked from commit b95035f)
CaseyPan pushed a commit to CaseyPan/druid that referenced this pull request Nov 17, 2023
for some exotic queries like:

  SELECT
  	'_'||dim1,
    MIN(cast(0 as double)) OVER (),
    MIN(cast((cnt||cnt) as bigint)) OVER ()
  FROM foo
the compilation have resulted in NPE -s mostly because VirtualColumn -s were not handled properly
riovic918data pushed a commit to riovic918data/druid that referenced this pull request Jun 12, 2026
for some exotic queries like:

  SELECT
  	'_'||dim1,
    MIN(cast(0 as double)) OVER (),
    MIN(cast((cnt||cnt) as bigint)) OVER ()
  FROM foo
the compilation have resulted in NPE -s mostly because VirtualColumn -s were not handled properly
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants