Skip to content

minor: add test to cover IPC arrow file read#1450

Merged
milenkovicm merged 2 commits intoapache:mainfrom
milenkovicm:minor_add_test_for_arrow_read
Feb 11, 2026
Merged

minor: add test to cover IPC arrow file read#1450
milenkovicm merged 2 commits intoapache:mainfrom
milenkovicm:minor_add_test_for_arrow_read

Conversation

@milenkovicm
Copy link
Contributor

Which issue does this PR close?

Closes #.

Rationale for this change

looks like arrow read is not supported, opened apache/datafusion#20280 to add proto support for it.

What changes are included in this PR?

  • created "unsupported" test
  • removed unsupported tests
  • clean up few minor things

Are there any user-facing changes?

@milenkovicm
Copy link
Contributor Author

@danielhumanmod, @mattcuento, @killzoner review would be appreciated if you have time

@milenkovicm milenkovicm force-pushed the minor_add_test_for_arrow_read branch from 93b9915 to f5be95e Compare February 10, 2026 21:53
#[case::standalone(standalone_context())]
#[case::remote(remote_context())]
#[tokio::test]
async fn should_execute_sql_app_name_show(
Copy link
Contributor

Choose a reason for hiding this comment

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

what's the reasoning on removing this one? Wondering since the table registered is in parquet

Copy link
Contributor

Choose a reason for hiding this comment

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

Nvm I see the sql is the exact same as should_execute_sql_collect. Was this case intended to assert on the job name/test session properties?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is another test which test app name i believe

Copy link
Contributor

@mattcuento mattcuento left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@killzoner killzoner left a comment

Choose a reason for hiding this comment

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

LGTM

Co-authored-by: jgrim <killzoner@users.noreply.github.com>
@milenkovicm milenkovicm changed the title minor: add test to cover arrow read minor: add test to cover IPC arrow file read Feb 11, 2026
@milenkovicm
Copy link
Contributor Author

thanks guys for reviews

@milenkovicm milenkovicm merged commit 5c83205 into apache:main Feb 11, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants