Skip to content

minor: use sql to setup test data for joins.slt rather than rust #6656

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 13, 2023

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Jun 13, 2023

Which issue does this PR close?

Follow on to #6642 from @jiangzhx
related to #6302

Rationale for this change

I noticed we could port more of the rust based setup code in #6642 to use sql which makes it clearer what is going on as well as is more concise

What changes are included in this PR?

Port 4 test setups from Rust to SQL

Are these changes tested?

covered by existing tests

Are there any user-facing changes?

no

@alamb alamb added the development-process Related to development process of DataFusion label Jun 13, 2023
@github-actions github-actions bot added core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) and removed development-process Related to development process of DataFusion labels Jun 13, 2023
@jiangzhx
Copy link
Contributor

Sorry for the late reply.
there are places similar to #6643, should we also migrate them from rs to SQL?

https://github.com/apache/arrow-datafusion/blob/071a2a6dfd43b3b225fce1af8838fa95c79f9ede/datafusion/core/tests/sqllogictests/src/main.rs#L266-L287

arrow_cast(c1, 'Date32') as c1,
arrow_cast(c2, 'Date64') as c2,
c3,
arrow_cast(c4, 'Dictionary(Int32, Utf8)') as c4
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, so that's how to cast a dictionary array.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 I think arrow_cast is the key if you want to control the arrow types specifically (otherwise you are stuck with the types that have an SQL mapping)

@alamb
Copy link
Contributor Author

alamb commented Jun 13, 2023

@jiangzhx

there are places similar to #6643, should we also migrate them from rs to SQL?

Yes, that would be awesome. Thank you.

@alamb alamb merged commit 2ba067c into apache:main Jun 13, 2023
@alamb alamb deleted the alamb/use_sql branch June 13, 2023 12:37
@jiangzhx
Copy link
Contributor

@jiangzhx

there are places similar to #6643, should we also migrate them from rs to SQL?

Yes, that would be awesome. Thank you.

OK, I will take the rest of it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core DataFusion crate sqllogictest SQL Logic Tests (.slt)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants