Skip to content

[fix](regression) Avoid prepared Arrow JDBC path in remote IP auth test#64024

Merged
morrySnow merged 1 commit into
apache:masterfrom
CalvinKirs:fix-master-arrow-auth-remote-ip-statement
Jun 3, 2026
Merged

[fix](regression) Avoid prepared Arrow JDBC path in remote IP auth test#64024
morrySnow merged 1 commit into
apache:masterfrom
CalvinKirs:fix-master-arrow-auth-remote-ip-statement

Conversation

@CalvinKirs
Copy link
Copy Markdown
Member

@CalvinKirs CalvinKirs commented Jun 2, 2026

What problem does this PR solve?

Related PR: 63506

Problem Summary: test_auth_remote_ip only needs to verify that Arrow Flight SQL remote IP authentication allows a matched user to run SELECT 1. The shared sql_impl helper uses PreparedStatement, and Arrow Flight SQL JDBC 17 can report a close-time 8-byte client allocator leak after the prepared path has already consumed the result. This changes the case to use JdbcUtils.executeQueryToList, which uses createStatement().executeQuery(...), so the test avoids the prepared statement cleanup path without ignoring conn.close() exceptions.

Release note

None

Check List (For Author)

  • Test: Manual test
    • bash -n run-regression-test.sh
    • /tmp/arrow-flight-close-repro JDBC statement path against 127.0.0.1:43080 repeated 10 times
    • ./run-regression-test.sh --run -d arrow_flight_sql_p0 -s test_auth_remote_ip -g arrow_flight_sql failed in the current local cluster at Arrow Flight SQL authentication: Access denied for user 'flight_auth_remote_ip_user@0.0.0.0'; it did not reach the SELECT 1/close path.
  • Behavior changed: No
  • Does this need documentation: No

### What problem does this PR solve?

Issue Number: None

Related PR: None

Problem Summary: test_auth_remote_ip only needs to verify that Arrow Flight SQL remote IP authentication allows SELECT 1 for the matched host. The shared sql_impl helper uses PreparedStatement, and Arrow Flight SQL JDBC 17 can report a close-time 8-byte client allocator leak after this prepared path has already consumed the result. Use the statement query helper for this simple SELECT 1 check to avoid the prepared statement cleanup path without ignoring close exceptions.

### Release note

None

### Check List (For Author)

- Test: Manual test

    - bash -n run-regression-test.sh

    - /tmp/arrow-flight-close-repro JDBC statement path against 127.0.0.1:43080 repeated 10 times

    - ./run-regression-test.sh --run -d arrow_flight_sql_p0 -s test_auth_remote_ip -g arrow_flight_sql (failed in current local cluster at Arrow Flight SQL authentication: Access denied for user 'flight_auth_remote_ip_user@0.0.0.0'; did not reach SELECT 1/close path)

- Behavior changed: No

- Does this need documentation: No
@hello-stephen
Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@CalvinKirs
Copy link
Copy Markdown
Member Author

run buildall

@CalvinKirs
Copy link
Copy Markdown
Member Author

/review

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Review result: no blocking issues found.

Critical checkpoint conclusions:

  • Goal and proof: The PR changes test_auth_remote_ip to execute the final SELECT 1 through JdbcUtils.executeQueryToList, avoiding the Arrow Flight SQL prepared-statement path while preserving the remote-IP auth assertion. The changed assertion still verifies one row with value 1. The PR reports manual coverage; I additionally checked the diff and git diff --check.
  • Scope and clarity: The change is small and focused: one import plus replacing sql_impl(conn, "SELECT 1") with the existing statement-query utility.
  • Concurrency and lifecycle: No new concurrency, shared state, lock ordering, static initialization, or special lifecycle management is introduced.
  • Configuration and compatibility: No config, protocol, storage format, FE-BE contract, or rolling-upgrade compatibility changes are introduced.
  • Parallel paths: Existing helpers already distinguish prepared execution from statement execution (executeToList vs executeQueryToList / arrow_flight_sql_no_prepared); this test intentionally switches only the affected single-query path.
  • Conditions and error handling: Existing cleanup behavior is preserved, including dropping test users in finally and not suppressing the connection close path under test.
  • Test coverage and results: This is a regression-test-only fix. No .out result changes are needed because the test uses assertions rather than generated query output.
  • Observability, transactions, persistence, writes, memory tracking: Not applicable to this test-only change.
  • Performance: Not relevant beyond avoiding the problematic prepared-statement path; no hot-path production code is changed.

User focus response: no additional user-provided review focus was present.

@morrySnow morrySnow merged commit 953a894 into apache:master Jun 3, 2026
36 checks passed
@CalvinKirs CalvinKirs deleted the fix-master-arrow-auth-remote-ip-statement branch June 3, 2026 03:36
github-actions Bot pushed a commit that referenced this pull request Jun 3, 2026
…st (#64024)

### What problem does this PR solve?

Related PR: [63506](#63506)

Problem Summary: `test_auth_remote_ip` only needs to verify that Arrow
Flight SQL remote IP authentication allows a matched user to run `SELECT
1`. The shared `sql_impl` helper uses `PreparedStatement`, and Arrow
Flight SQL JDBC 17 can report a close-time 8-byte client allocator leak
after the prepared path has already consumed the result. This changes
the case to use `JdbcUtils.executeQueryToList`, which uses
`createStatement().executeQuery(...)`, so the test avoids the prepared
statement cleanup path without ignoring `conn.close()` exceptions.
github-actions Bot pushed a commit that referenced this pull request Jun 3, 2026
…st (#64024)

### What problem does this PR solve?

Related PR: [63506](#63506)

Problem Summary: `test_auth_remote_ip` only needs to verify that Arrow
Flight SQL remote IP authentication allows a matched user to run `SELECT
1`. The shared `sql_impl` helper uses `PreparedStatement`, and Arrow
Flight SQL JDBC 17 can report a close-time 8-byte client allocator leak
after the prepared path has already consumed the result. This changes
the case to use `JdbcUtils.executeQueryToList`, which uses
`createStatement().executeQuery(...)`, so the test avoids the prepared
statement cleanup path without ignoring `conn.close()` exceptions.
yiguolei pushed a commit that referenced this pull request Jun 3, 2026
…e IP auth test #64024 (#64049)

Cherry-picked from #64024

Co-authored-by: Calvin Kirs <guoqiang@selectdb.com>
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.

4 participants