Skip to content

Conversation

@guan404ming
Copy link
Member

Related Issue

Why

get_pandas_df deprecated in #48875, which would be replaced by get_df. Thus, we need to migrate them in providers.

How

After investigation, I found that ElasticsearchSQLHook has its own ElasticsearchSQLCursor which is not compatable with polars cursor execution since there is a abstraction for cursor execution when calling polars.read_database.

reference: polars.read_database and the abstraction called _executor _executor

Also there is nothing about the integration info on polars IO document and Elasticsearch document thus I thought it would be better to raise NotImplementedError for it like we did in #50017.

Minor change: migrate to common-sql 1.27.0 to have better get_df type def


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@guan404ming
Copy link
Member Author

This is the last one of the whole migration!

@guan404ming
Copy link
Member Author

guan404ming commented May 11, 2025

BTW should we also bump other packages using get_df to common-sql=1.27.0 to have better type definition?

@eladkal
Copy link
Contributor

eladkal commented May 11, 2025

BTW should we also bump other packages using get_df to common-sql=1.27.0 to have better type definition?

We bump min version when there is a reason.. otherwise we will always have latest version as minimum.

@guan404ming
Copy link
Member Author

Sure, thanks for the answer and it does make sense. Also thanks for the review and suggestions.

@eladkal eladkal merged commit 5cc14d1 into apache:main May 11, 2025
64 checks passed
@guan404ming guan404ming deleted the migrate-elasticsearch branch May 11, 2025 13:25
@guan404ming
Copy link
Member Author

Thanks!

sanederchik pushed a commit to sanederchik/airflow that referenced this pull request Jun 7, 2025
* Migrate `ElasticsearchSQLHook` to use `get_df`

* Add todo comment
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.

2 participants