Skip to content

ARROW-14519: [C++] Properly error if joining on unsupported type#11625

Closed
lidavidm wants to merge 2 commits intoapache:masterfrom
lidavidm:arrow-14519
Closed

ARROW-14519: [C++] Properly error if joining on unsupported type#11625
lidavidm wants to merge 2 commits intoapache:masterfrom
lidavidm:arrow-14519

Conversation

@lidavidm
Copy link
Copy Markdown
Member

@lidavidm lidavidm commented Nov 5, 2021

Instead of DCHECK, return a NotImplemented.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 5, 2021

@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 5, 2021

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

@lidavidm
Copy link
Copy Markdown
Member Author

lidavidm commented Nov 5, 2021

CC @michalursa. Note that the KeyColumnMetadata computed here is unused, but computing it here serves as a useful up-front check as otherwise we run into a DCHECK later during plan execution instead, so I opted to keep it.

@lidavidm
Copy link
Copy Markdown
Member Author

lidavidm commented Nov 5, 2021

Moved the validation to HashJoinSchema::ValidateSchemas as suggested on JIRA.

@lidavidm
Copy link
Copy Markdown
Member Author

lidavidm commented Nov 5, 2021

Also includes a fix for the build issues at HEAD. see #11627

Copy link
Copy Markdown
Contributor

@michalursa michalursa left a comment

Choose a reason for hiding this comment

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

I realized that KeyColumnMetadata was not needed. The reason it exists is that it was meant for future changes to hash join and I forgot to remove it in the original hash join change.

@lidavidm lidavidm closed this in 412da89 Nov 6, 2021
@lidavidm lidavidm deleted the arrow-14519 branch November 6, 2021 13:49
@ursabot
Copy link
Copy Markdown

ursabot commented Nov 6, 2021

Benchmark runs are scheduled for baseline = ae808e0 and contender = 412da89. 412da89 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Finished ⬇️0.51% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.76% ⬆️0.04%] ursa-thinkcentre-m75q
Supported benchmarks:
ursa-i9-9960x: langs = Python, R, JavaScript
ursa-thinkcentre-m75q: langs = C++, Java
ec2-t3-xlarge-us-east-2: cloud = True

kou pushed a commit that referenced this pull request Nov 10, 2021
Instead of DCHECK, return a NotImplemented.

Closes #11625 from lidavidm/arrow-14519

Authored-by: David Li <li.davidm96@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
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.

3 participants