Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Introducerow_limit param#607

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
varun-edachali-dbx merged 13 commits intosea-migrationfrommax-rows-sea
Jul 7, 2025
Merged

Conversation

@varun-edachali-dbx
Copy link
Contributor

@varun-edachali-dbxvarun-edachali-dbx commentedJun 19, 2025•
edited
Loading

What type of PR is this?

  • Feature

Description

Introduces arow_limit param in theCursor that constrains the number of rows in the query result, throughresultRowLimit in Thrift and throughmax_rows in SEA.

Despiterow_limit not existingin the spec, we feel it is reasonable to introduce it as a param in theCursor, because:

  • theConnection class has a number of non-spec params
  • a risk of non-spec methods is the feature not being discovered (i.e., users not realising this param is available to them), but this is not a major issue, since the feature can effectively be replicated by includingLIMIT in their SQL query.

How is this tested?

  • Unit tests
  • E2E Tests
  • Manually
  • N/A

Related Tickets & Documents

N/A

Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
@varun-edachali-dbxvarun-edachali-dbx changed the titleIntroducemax_rows param forCursor initialisation forSEA backendIntroducemax_rows param inCursor constructor to be used bySEA backendJun 19, 2025
@varun-edachali-dbxvarun-edachali-dbx changed the titleIntroducemax_rows param inCursor constructor to be used bySEA backendIntroducemax_rows param inCursor constructor to be used by SEA backendJun 19, 2025
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
@varun-edachali-dbxvarun-edachali-dbx changed the titleIntroducemax_rows param inCursor constructor to be used by SEA backendIntroducerow_limit paramJun 19, 2025
@varun-edachali-dbxvarun-edachali-dbx marked this pull request as draftJune 19, 2025 07:58
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
@varun-edachali-dbxvarun-edachali-dbx marked this pull request as ready for reviewJune 20, 2025 03:50
@varun-edachali-dbxvarun-edachali-dbx marked this pull request as draftJune 20, 2025 04:00
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
@jayantsing-db
Copy link
Contributor

is this a breaking change in thrift? the thrift users would have to change to row_limit?

@varun-edachali-dbx
Copy link
ContributorAuthor

is this a breaking change in thrift? the thrift users would have to change to row_limit?

There is currently no way for the user to specify a row limit with the Thrift backend. The newly introducedrow_limit will setresultRowLimit inTExecuteStatementReq and allow the user to do so. It is not a breaking change because this defaults toNone, which would maintain current behaviour.

In the current implementation,arraysize is set as themax_rows for direct results in case of Thrift. It does not constrain the total rows in the result.row_limit andarraysize will serve distinct purposes.

Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
@varun-edachali-dbxvarun-edachali-dbx merged commit4f11ff0 intosea-migrationJul 7, 2025
23 checks passed
@varun-edachali-dbxvarun-edachali-dbx mentioned this pull requestJul 22, 2025
5 tasks
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@shivam2680shivam2680shivam2680 left review comments

@jayantsing-dbjayantsing-dbjayantsing-db approved these changes

@deeksha-dbdeeksha-dbAwaiting requested review from deeksha-db

@samikshya-dbsamikshya-dbAwaiting requested review from samikshya-db

@jprakash-dbjprakash-dbAwaiting requested review from jprakash-db

@jackyhu-dbjackyhu-dbAwaiting requested review from jackyhu-db

@madhav-dbmadhav-dbAwaiting requested review from madhav-db

@gopalldbgopalldbAwaiting requested review from gopalldb

@vikrantpuppalavikrantpuppalaAwaiting requested review from vikrantpuppala

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@varun-edachali-dbx@jayantsing-db@shivam2680
[8]ページ先頭
©2009-2025 Movatter.jp