Skip to content

Conversation

@xu20160924
Copy link
Contributor

@xu20160924 xu20160924 commented Dec 9, 2025

What changes were proposed in this pull request?

Add getString test coverage for BINARY type with UTF-8 encoding in "get binary type by column label" test

Why are the changes needed?

Test coverage

Does this PR introduce any user-facing change?

No

How was this patch tested?

./build/sbt -Phive "connect-client-jdbc/testOnly *SparkConnectJdbcDataTypeSuite"
[info] Run completed in 26 seconds, 574 milliseconds.
[info] Total number of tests run: 21
[info] Suites: completed 1, aborted 0
[info] Tests: succeeded 21, failed 0, canceled 0, ignored 0, pending 0
[info] All tests passed.

Was this patch authored or co-authored using generative AI tooling?

No

…s to imports in SparkConnectJdbcDataTypeSuite
@xu20160924
Copy link
Contributor Author

#53385 (comment) I create new pull request based on the new commits.

Copy link
Member

@pan3793 pan3793 left a comment

Choose a reason for hiding this comment

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

LGTM, cc @LuciferYang

LuciferYang
LuciferYang previously approved these changes Dec 10, 2025
@LuciferYang LuciferYang dismissed their stale review December 10, 2025 08:09

This is not mandatory; there are numerous similar cases in the current codebase.

assert(bytes.sameElements(testBytes2))
assert(!rs.wasNull)

val stringValue = rs.getString("test_binary")
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these 3 lines would be fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I restore to only leave these three lines.

@LuciferYang
Copy link
Contributor

This is not mandatory; there are numerous similar cases in the current codebase.

image

…s to imports in SparkConnectJdbcDataTypeSuite
@xu20160924 xu20160924 changed the title [SPARK-54629][CONNECT][TEST] Refactor fully-qualified class references to imports in SparkConnectJdbcDataTypeSuite [SPARK-54629][CONNECT][TEST] Supplement getString with BINARY type (UTF-8) test Dec 11, 2025
Copy link
Contributor

@LuciferYang LuciferYang left a comment

Choose a reason for hiding this comment

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

+1, LGTM(pending tests)

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