Skip to content

Keep the existing default catalog for SessionStateBuilder::new_from_existing#11991

Merged
alamb merged 1 commit intoapache:mainfrom
goldmedal:feature/11988-keep-exist-default-catalog
Aug 15, 2024
Merged

Keep the existing default catalog for SessionStateBuilder::new_from_existing#11991
alamb merged 1 commit intoapache:mainfrom
goldmedal:feature/11988-keep-exist-default-catalog

Conversation

@goldmedal
Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Closes #11988 .

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@github-actions github-actions Bot added the core Core DataFusion crate label Aug 14, 2024
Copy link
Copy Markdown
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thanks @goldmedal -- this makes sense to me

.unwrap()
.table_exist("employee"));

// if `with_create_default_catalog_and_schema` is disabled, the new one shouldn't create default catalog and schema
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.

I ran this test locally without the code changes in this PR and the test fails (as expected) thus verifying test coverage for the new code 👍


assertion failed: new_state.catalog_list().catalog(default_catalog.as_str()).unwrap().schema(default_schema.as_str()).unwrap().table_exist("employee")
thread 'execution::session_state::tests::test_from_existing' panicked at datafusion/core/src/execution/session_state.rs:1887:9:
assertion failed: new_state.catalog_list().catalog(default_catalog.as_str()).unwrap().schema(default_schema.as_str()).unwrap().table_exist("employee")
stack backtrace:
   0: rust_begin_unwind
             at /rustc/3f5fd8dd41153bc5fdca9427e9e05be2c767ba23/library/std/src/panicking.rs:652:5
   1: core::panicking::panic_fmt
             at /rustc/3f5fd8dd41153bc5fdca9427e9e05be2c767ba23/library/core/src/panicking.rs:72:14
   2: core::panicking::panic
             at /rustc/3f5fd8dd41153bc5fdca9427e9e05be2c767ba23/library/core/src/panicking.rs:146:5
   3: datafusion::execution::session_state::tests::test_from_existing
             at ./src/execution/session_state.rs:1887:9
   4: datafusion::execution::session_state::tests::test_from_existing::{{closure}}
             at ./src/execution/session_state.rs:1848:32
   5: core::ops::function::FnOnce::call_once
             at /rustc/3f5fd8dd41153bc5fdca9427e9e05be2c767ba23/library/core/src/ops/function.rs:250:5
   6: core::ops::function::FnOnce::call_once
             at /rustc/3f5fd8dd41153bc5fdca9427e9e05be2c767ba23/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

@alamb alamb merged commit cb3ec77 into apache:main Aug 15, 2024
@alamb
Copy link
Copy Markdown
Contributor

alamb commented Aug 15, 2024

Thanks again @goldmedal

@goldmedal goldmedal deleted the feature/11988-keep-exist-default-catalog branch August 15, 2024 17:44
@goldmedal
Copy link
Copy Markdown
Contributor Author

Thanks @alamb ❤️

Friede80 pushed a commit to massive-com/arrow-datafusion that referenced this pull request Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Disable create_default_catalog when the exist session state has default catalog for SessionStateBuilder

2 participants