Skip to content

Implement ResultSet Abstraction (backend interfaces for fetch phase) #574

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

Merged
merged 344 commits into from
Jun 3, 2025

Conversation

varun-edachali-dbx
Copy link
Collaborator

@varun-edachali-dbx varun-edachali-dbx commented May 26, 2025

What type of PR is this?

  • Refactor

Description

  • Create ResultSet interface and make the existing ResultSet (as ThriftResultSet) implement it. Concrete Thrift Backend layer should return ThriftResultSet from execute commands.
  • Generalise the return types of some functions in the backend interface to prevent alignment with Thrift (create new Status enum to avoid using Thrift's Status type).

How is this tested?

  • Unit tests
    • some of the existing unit tests were slightly altered to account for the introduction of the interface. No unit tests were removed or introduced.
  • E2E Tests
  • Manually
  • N/A

Related Tickets & Documents

https://docs.google.com/document/d/1Y-eXLhNqqhrMVGnOlG8sdFrCxBTN1GdQvuKG4IfHmo0/edit?usp=sharing
https://databricks.atlassian.net/browse/PECOBLR-445?atlOrigin=eyJpIjoiMjA3ZTljYTUzNDgxNDY4MmFhMThjYjI5MWY0Yjc0MTIiLCJwIjoiaiJ9

Copy link

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

Copy link

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

Copy link

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

Copy link

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

varun-edachali-dbx and others added 16 commits May 28, 2025 03:40
Signed-off-by: varun-edachali-dbx <[email protected]>
Signed-off-by: varun-edachali-dbx <[email protected]>
Signed-off-by: varun-edachali-dbx <[email protected]>
Signed-off-by: varun-edachali-dbx <[email protected]>
new codeowners

Signed-off-by: varun-edachali-dbx <[email protected]>
…t to prevent server side resource leaks (#554)

* Enhance Cursor close handling and context manager exception management

* tests

* fmt

* Fix Cursor.close() to properly handle CursorAlreadyClosedError

* Remove specific test message from Cursor.close() error handling

* Improve error handling in connection and cursor context managers to ensure proper closure during exceptions, including KeyboardInterrupt. Add tests for nested cursor management and verify operation closure on server-side errors.

* add

* add

Signed-off-by: varun-edachali-dbx <[email protected]>
* PECOBLR-86 Improve logging for debug level

Signed-off-by: Sai Shree Pradhan <[email protected]>

* PECOBLR-86 Improve logging for debug level

Signed-off-by: Sai Shree Pradhan <[email protected]>

* fixed format

Signed-off-by: Sai Shree Pradhan <[email protected]>

* used lazy logging

Signed-off-by: Sai Shree Pradhan <[email protected]>

* changed debug to error logs

Signed-off-by: Sai Shree Pradhan <[email protected]>

* used lazy logging

Signed-off-by: Sai Shree Pradhan <[email protected]>

---------

Signed-off-by: Sai Shree Pradhan <[email protected]>
Signed-off-by: varun-edachali-dbx <[email protected]>
More conditions to run github actions

Signed-off-by: varun-edachali-dbx <[email protected]>
* Modified the gitignore file to not have .idea file

* [PECO-1803] Splitting the PySql connector into the core and the non core part (#417)

* Implemented ColumnQueue to test the fetchall without pyarrow

Removed token

removed token

* order of fields in row corrected

* Changed the folder structure and tested the basic setup to work

* Refractored the code to make connector to work

* Basic Setup of connector, core and sqlalchemy is working

* Basic integration of core, connect and sqlalchemy is working

* Setup working dynamic change from ColumnQueue to ArrowQueue

* Refractored the test code and moved to respective folders

* Added the unit test for column_queue

Fixed __version__

Fix

* venv_main added to git ignore

* Added code for merging columnar table

* Merging code for columnar

* Fixed the retry_close sesssion test issue with logging

* Fixed the databricks_sqlalchemy tests and introduced pytest.ini for the sqla_testing

* Added pyarrow_test mark on pytest

* Fixed databricks.sqlalchemy to databricks_sqlalchemy imports

* Added poetry.lock

* Added dist folder

* Changed the pyproject.toml

* Minor Fix

* Added the pyarrow skip tag on unit tests and tested their working

* Fixed the Decimal and timestamp conversion issue in non arrow pipeline

* Removed not required files and reformatted

* Fixed test_retry error

* Changed the folder structure to src / databricks

* Removed the columnar non arrow flow to another PR

* Moved the README to the root

* removed columnQueue instance

* Revmoved databricks_sqlalchemy dependency in core

* Changed the pysql_supports_arrow predicate, introduced changes in the pyproject.toml

* Ran the black formatter with the original version

* Extra .py removed from all the __init__.py files names

* Undo formatting check

* Check

* Check

* Check

* Check

* Check

* Check

* Check

* Check

* Check

* Check

* Check

* Check

* Check

* Check

* BIG UPDATE

* Refeactor code

* Refractor

* Fixed versioning

* Minor refractoring

* Minor refractoring

* Changed the folder structure such that sqlalchemy has not reference here

* Fixed README.md and CONTRIBUTING.md

* Added manual publish

* On push trigger added

* Manually setting the publish step

* Changed versioning in pyproject.toml

* Bumped up the version to 4.0.0.b3 and also changed the structure to have pyarrow as optional

* Removed the sqlalchemy tests from integration.yml file

* [PECO-1803] Print warning message if pyarrow is not installed (#468)

Print warning message if pyarrow is not installed

Signed-off-by: Jacky Hu <[email protected]>

* [PECO-1803] Remove sqlalchemy and update README.md (#469)

Remove sqlalchemy and update README.md

Signed-off-by: Jacky Hu <[email protected]>

* Removed all sqlalchemy related stuff

* generated the lock file

* Fixed failing tests

* removed poetry.lock

* Updated the lock file

* Fixed poetry numpy 2.2.2 issue

* Workflow fixes

---------

Signed-off-by: Jacky Hu <[email protected]>
Co-authored-by: Jacky Hu <[email protected]>
Signed-off-by: varun-edachali-dbx <[email protected]>
* Removed python3.8 support

* Minor fix

Signed-off-by: varun-edachali-dbx <[email protected]>
Support for Py till 3.12

Signed-off-by: varun-edachali-dbx <[email protected]>
* Increased the number of retry attempts allowed  (#486)

Updated the number of attempts allowed

* bump version to 3.7.1 (#487)

bumped up version

* Refractore

* Minor change

Signed-off-by: varun-edachali-dbx <[email protected]>
bumped up the version

Signed-off-by: varun-edachali-dbx <[email protected]>
Signed-off-by: varun-edachali-dbx <[email protected]>
Copy link

github-actions bot commented Jun 2, 2025

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

Signed-off-by: varun-edachali-dbx <[email protected]>
Copy link

github-actions bot commented Jun 2, 2025

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

Copy link

github-actions bot commented Jun 2, 2025

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

Signed-off-by: varun-edachali-dbx <[email protected]>
Copy link

github-actions bot commented Jun 2, 2025

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

Signed-off-by: varun-edachali-dbx <[email protected]>
Copy link

github-actions bot commented Jun 2, 2025

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

Signed-off-by: varun-edachali-dbx <[email protected]>
Copy link

github-actions bot commented Jun 3, 2025

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

Signed-off-by: varun-edachali-dbx <[email protected]>
Copy link

github-actions bot commented Jun 3, 2025

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

Copy link

github-actions bot commented Jun 3, 2025

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

Copy link

github-actions bot commented Jun 3, 2025

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

Copy link
Contributor

@jayantsing-db jayantsing-db left a comment

Choose a reason for hiding this comment

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

thanks. a minor comment for a separate PR.

return ThriftResultSet(
connection=cursor.connection,
execute_response=execute_response,
thrift_client=self,
Copy link
Contributor

Choose a reason for hiding this comment

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

we already pass connection which has session which in turn has the backend? can we look into normalising this init params (in a separate PR)

@varun-edachali-dbx varun-edachali-dbx merged commit 3c78ed7 into sea-migration Jun 3, 2025
22 of 23 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.