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

Feature/infer schema upstream #43

Closed
wants to merge 2 commits into from

Conversation

alihan-synnada
Copy link

Which issue does this PR close?

No related issue

Rationale for this change

I came across the following obstacles trying to write some custom schema inference code:

  1. There are cases where we need to know if the user explicitly set the schema_infer_max_rec option.
  2. The stream passed into read_to_delimited_chunks_from_stream does not necessarily have to be BoxStream<'static>

What changes are included in this PR?

  1. Make schema_infer_max_rec an Option
  2. Add lifetime parameter to functions in CSV and compression that makes the compiler complain

Are these changes tested?

Tested with existing code and tests.

Are there any user-facing changes?

  1. CsvOptions::schema_infer_max_rec and JsonOptions::schema_infer_max_rec are now Option<usize>s as opposed to usize (with_schema_infer_max_rec signature is unchanged)
  2. read_to_delimited_chunks, read_to_delimited_chunks_from_stream, convert_stream, convert_to_compress_stream now take lifetime parameters

I believe (1) is a breaking change but I'm not sure about (2)

Copy link
Collaborator

@ozankabak ozankabak left a comment

Choose a reason for hiding this comment

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

LGTM

@berkaysynnada
Copy link
Collaborator

Merged to upstream

gokselk pushed a commit to gokselk/datafusion-upstream that referenced this pull request Nov 7, 2024
…regation (apache#12946)

* Improve unparsing for ORDER BY with Aggregation functions (synnada-ai#38)

* Improve UNION unparsing (synnada-ai#39)

* Scalar functions in ORDER BY unparsing support (synnada-ai#41)

* Improve unparsing for complex Window functions with Aggregation (synnada-ai#42)

* WindowFunction order_by should respect `supports_nulls_first_in_sort` dialect setting (synnada-ai#43)

* Fix plan_to_sql

* Improve
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants