Skip to content

feat: Support configuring datafusion catalog in sqllogictest framework #1865

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

feat: Support configuring datafusion catalog in sqllogictest framework #1865

wants to merge 10 commits into from

Conversation

@kaori-seasons
Copy link

@kaori-seasons kaori-seasons commented Nov 17, 2025

Which issue does this PR close?

Related to issue-1780

  • Closes #.

What changes are included in this PR?

Are these changes tested?

@liurenjie1024
Copy link
Contributor

liurenjie1024 commented Nov 26, 2025

Hi @kaori-seasons this pr contains a lot of unrelated changes, please fix format first.

@kaori-seasons
Copy link
Author

kaori-seasons commented Nov 27, 2025 •
edited
Loading

Hi @kaori-seasons this pr contains a lot of unrelated changes, please fix format first.

Hello @liurenjie1024 , these seemingly unrelated changes were caused by my local use of cargo fmt, and I'm fixing them.

@lliangyu-lin
Copy link
Contributor

lliangyu-lin commented Dec 3, 2025 •
edited
Loading

Hi @kaori-seasons, thanks for helping out! My original intention of the issue is to keep it simple for the first version of sql logic test framework and iterate from there.
My thought on the design of catalog in the schedule file would be something like

[catalog]
[CATALOG_NAME] = {
    type = "[CATALOG_TYPE]",
    properties = {
        key1 = val1,
        key2 = val2,
        ...
    }
}

and we don't need to specify which engine uses which catalog to keep things simpler (I think most test scenarios just needs one shared catalog).

Xuanwo
Xuanwo reviewed Dec 3, 2025
View reviewed changes
crates/catalog/glue/src/error.rs
/// Format AWS SDK error into iceberg error
pub(crate) fn from_aws_sdk_error<T>(error: aws_sdk_glue::error::SdkError<T>) -> Error
where T: Debug {
where
Copy link
Member

@Xuanwo Xuanwo Dec 3, 2025

Choose a reason for hiding this comment

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

Why this change?

crates/catalog/hms/tests/hms_catalog_test.rs
"my_table".to_string()
)]);
assert_eq!(
result,
Copy link
Member

@Xuanwo Xuanwo Dec 3, 2025

Choose a reason for hiding this comment

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

There are many format related changes, it's better to avoid that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Xuanwo Xuanwo Xuanwo left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kaori-seasons @liurenjie1024 @lliangyu-lin @Xuanwo