Skip to content
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

fix: querying on date partitions (fixes #1445) #1481

Closed
wants to merge 10 commits into from

Conversation

ChewingGlass
Copy link
Contributor

Description

Fix querying on date partitions

Related Issue(s)

@github-actions
Copy link

ACTION NEEDED

delta-rs follows the Conventional Commits specification for release automation.

The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification.

@ChewingGlass ChewingGlass changed the title Fix querying on date partitions (fixes #1445) fix: querying on date partitions (fixes #1445) Jun 20, 2023
@@ -844,7 +844,7 @@ async fn test_issue_1292_datafusion_sql_projection() -> Result<()> {
#[tokio::test]
async fn test_issue_1291_datafusion_sql_partitioned_data() -> Result<()> {
let ctx = SessionContext::new();
let table = deltalake::open_table("./tests/data/http_requests")
let table = deltalake::open_table("./tests/data/issue_1291")
Copy link
Collaborator

@wjones127 wjones127 Jun 20, 2023

Choose a reason for hiding this comment

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

Instead of committing a table to the repo, could we write one in a temp directory? I think we want to reserve committing tables to ones that have to be generated by outside tools like Spark.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ChewingGlass you willing to update this part soon?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed it

Copy link
Collaborator

Choose a reason for hiding this comment

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

There seem to be some tests that need to be updated so they no longer expect dictionary encoded columns.
Also need to add the setup where you create a temporary directory and write the partitioned table :)

Copy link
Contributor

@watfordkcf watfordkcf left a comment

Choose a reason for hiding this comment

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

Been using this in our delta-rs deployments and it is working well.

@watfordkcf
Copy link
Contributor

What can I do to help move this along? We're currently pinning on a copy of this PR plus some merges and would very much like to move to a more official version.

@roeap
Copy link
Collaborator

roeap commented Aug 17, 2023

@watfordkcf - seems like there are some tests that need fixing. Haven't had a chance to properly review yet, but it seems they are partially related to a table that was previously commited to the (within this PR) and later removed not being created within the tests.

Other failures are related to column encodings. On first glance related to how we encode the partitioned columns.

@wjones127
Copy link
Collaborator

Closing since we merged #1594

@wjones127 wjones127 closed this Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
binding/rust Issues for the Rust crate rust
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to query by partition column
6 participants