Backport to branch(3) : Add JDBC database permission test #2913

feeblefakie · 2025-07-28T05:02:35Z

This is an automated backport of the following:

Original PR: Add JDBC database permission test #2896
Backported commit: d1fc213

Please merge this PR after all checks have passed.

gemini-code-assist

Summary of Changes

Hello @feeblefakie, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request backports a set of changes aimed at enhancing the testing infrastructure for the JDBC storage layer. Specifically, it introduces new integration tests designed to thoroughly verify database user and administrative permissions, ensuring robust security and access control for JDBC-backed operations.

Highlights

New JDBC Permission Tests: Added JdbcPermissionIntegrationTest and JdbcAdminPermissionIntegrationTest to validate user and administrative permissions within the JDBC storage component.
Dedicated Gradle Test Suite: Configured a new integrationTestJdbcPermission Gradle source set and task to isolate and run these specific permission-related integration tests.
JDBC User Management Utilities: Introduced JdbcPermissionTestUtils to programmatically create, drop, and grant necessary permissions to database users for testing purposes across different RDBMS (MySQL, Oracle, PostgreSQL, SQL Server).
Enhanced JDBC Environment Configuration: Updated JdbcEnv to support defining and retrieving properties for a 'normal user,' facilitating testing scenarios with restricted privileges.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request is an automated backport to add JDBC database permission tests. The changes include updates to the build.gradle file to create a new test suite, and new Java classes for the integration tests. The implementation for handling user and permission management for different RDB engines in JdbcPermissionTestUtils.java is mostly correct, but I've identified a few areas for improvement regarding the robustness of SQL statements for SQL Server. Specifically, user-provided names should be properly quoted to prevent potential SQL errors.

gemini-code-assist · 2025-07-28T05:04:36Z

core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcPermissionTestUtils.java

+      return String.format(
+          "CREATE LOGIN %s WITH PASSWORD = '%s', DEFAULT_DATABASE = master , CHECK_POLICY = OFF, CHECK_EXPIRATION = OFF; CREATE USER %s FOR LOGIN %s",
+          userName, password, userName, userName);


For SQL Server, the userName is not quoted in the generated SQL statement. This can lead to SQL errors if the username is a reserved keyword or contains special characters. It's safer to enclose identifiers in square brackets [].

Additionally, there's an extra space before the comma in master , CHECK_POLICY which could be removed for better readability and consistency.

Suggested change

return String.format(

"CREATE LOGIN %s WITH PASSWORD = '%s', DEFAULT_DATABASE = master , CHECK_POLICY = OFF, CHECK_EXPIRATION = OFF; CREATE USER %s FOR LOGIN %s",

userName, password, userName, userName);

return String.format(

"CREATE LOGIN [%s] WITH PASSWORD = '%s', DEFAULT_DATABASE = master, CHECK_POLICY = OFF, CHECK_EXPIRATION = OFF; CREATE USER [%s] FOR LOGIN [%s]",

userName, password, userName, userName);

gemini-code-assist · 2025-07-28T05:04:37Z

core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcPermissionTestUtils.java

+    } else if (JdbcTestUtils.isSqlite(rdbEngine)) {
+      throw new UnsupportedOperationException("SQLite does not support user management");
+    } else if (JdbcTestUtils.isSqlServer(rdbEngine)) {
+      return String.format("DROP USER %s; DROP LOGIN %s", userName, userName);


For SQL Server, the userName is not quoted in the DROP USER and DROP LOGIN statements. This can lead to SQL errors if the username is a reserved keyword or contains special characters. To improve robustness, it's recommended to enclose the username in square brackets [].

Suggested change

return String.format("DROP USER %s; DROP LOGIN %s", userName, userName);

return String.format("DROP USER [%s]; DROP LOGIN [%s]", userName, userName);

gemini-code-assist · 2025-07-28T05:04:37Z

core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcPermissionTestUtils.java

+      return new String[] {
+        String.format("ALTER ROLE [db_ddladmin] ADD MEMBER %s", userName),
+        String.format("ALTER ROLE [db_datareader] ADD MEMBER %s", userName),
+        String.format("ALTER ROLE [db_datawriter] ADD MEMBER %s", userName)
+      };


For SQL Server, the userName is not quoted when adding it as a member to database roles. This can cause SQL errors if the username is a reserved keyword or contains special characters. It's safer to enclose the username in square brackets [] to ensure the statement is robust.

Suggested change

return new String[] {

String.format("ALTER ROLE [db_ddladmin] ADD MEMBER %s", userName),

String.format("ALTER ROLE [db_datareader] ADD MEMBER %s", userName),

String.format("ALTER ROLE [db_datawriter] ADD MEMBER %s", userName)

};

return new String[] {

String.format("ALTER ROLE [db_ddladmin] ADD MEMBER [%s]", userName),

String.format("ALTER ROLE [db_datareader] ADD MEMBER [%s]", userName),

String.format("ALTER ROLE [db_datawriter] ADD MEMBER [%s]", userName)

};

KodaiD · 2025-07-28T07:02:47Z

Verified in https://github.com/scalar-labs/scalardb/actions/runs/16561973782.

Add JDBC database permission test (#2896)

0def825

feeblefakie assigned KodaiD Jul 28, 2025

gemini-code-assist bot reviewed Jul 28, 2025

View reviewed changes

KodaiD merged commit dc0a693 into 3 Jul 28, 2025
73 checks passed

KodaiD deleted the 3-pull-2896 branch July 28, 2025 07:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Backport to branch(3) : Add JDBC database permission test #2913

Backport to branch(3) : Add JDBC database permission test #2913

Uh oh!

feeblefakie commented Jul 28, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Jul 28, 2025

Uh oh!

gemini-code-assist bot Jul 28, 2025

Uh oh!

gemini-code-assist bot Jul 28, 2025

Uh oh!

KodaiD commented Jul 28, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	return String.format("DROP USER %s; DROP LOGIN %s", userName, userName);
	return String.format("DROP USER [%s]; DROP LOGIN [%s]", userName, userName);

Backport to branch(3) : Add JDBC database permission test #2913

Backport to branch(3) : Add JDBC database permission test #2913

Uh oh!

Conversation

feeblefakie commented Jul 28, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

KodaiD commented Jul 28, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants