You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "AMashenkov (via GitHub)" <gi...@apache.org> on 2025/07/24 11:49:45 UTC
[PR] IGNTIE-25588 Sql. Support CURRENT_USER function [ignite-3]
AMashenkov opened a new pull request, #6314:
URL: https://github.com/apache/ignite-3/pull/6314
Thank you for submitting the pull request.
To streamline the review process of the patch and ensure better code quality
we ask both an author and a reviewer to verify the following:
### The Review Checklist
- [ ] **Formal criteria:** TC status, codestyle, mandatory documentation. Also make sure to complete the following:
\- There is a single JIRA ticket related to the pull request.
\- The web-link to the pull request is attached to the JIRA ticket.
\- The JIRA ticket has the Patch Available state.
\- The description of the JIRA ticket explains WHAT was made, WHY and HOW.
\- The pull request title is treated as the final commit message. The following pattern must be used: IGNITE-XXXX Change summary where XXXX - number of JIRA issue.
- [ ] **Design:** new code conforms with the design principles of the components it is added to.
- [ ] **Patch quality:** patch cannot be split into smaller pieces, its size must be reasonable.
- [ ] **Code quality:** code is clean and readable, necessary developer documentation is added if needed.
- [ ] **Tests code quality:** test set covers positive/negative scenarios, happy/edge cases. Tests are effective in terms of execution time and resources.
### Notes
- [Apache Ignite Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Java+Code+Style+Guide)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25588 Sql. Support CURRENT_USER function [ignite-3]
Posted by "AMashenkov (via GitHub)" <gi...@apache.org>.
AMashenkov commented on code in PR #6314:
URL: https://github.com/apache/ignite-3/pull/6314#discussion_r2244674360
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/fsm/QueryExecutor.java:
##########
@@ -176,6 +177,10 @@ public CompletableFuture<AsyncSqlCursor<InternalSqlRow>> executeQuery(
@Nullable CancellationToken cancellationToken,
Object... params
) {
+ if (properties.userName() == null) {
+ properties.userName(Commons.SYSTEM_USER_NAME);
Review Comment:
Removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25588 Sql. Support CURRENT_USER function [ignite-3]
Posted by "AMashenkov (via GitHub)" <gi...@apache.org>.
AMashenkov commented on code in PR #6314:
URL: https://github.com/apache/ignite-3/pull/6314#discussion_r2241925135
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/api/IgniteSqlImpl.java:
##########
@@ -660,7 +661,8 @@ private static SqlProperties toPropertiesBuilder(Statement statement) {
return new SqlProperties()
.timeZoneId(statement.timeZoneId())
.defaultSchema(IgniteNameUtils.parseIdentifier(statement.defaultSchema()))
- .queryTimeout(statement.queryTimeout(TimeUnit.MILLISECONDS));
+ .queryTimeout(statement.queryTimeout(TimeUnit.MILLISECONDS))
+ .userName(Commons.SYSTEM_USER_NAME);
Review Comment:
No, we set name SYSTEM for embedded case only. In other cases, default user name is "unknown".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25588 Sql. Support CURRENT_USER function [ignite-3]
Posted by "AMashenkov (via GitHub)" <gi...@apache.org>.
AMashenkov commented on code in PR #6314:
URL: https://github.com/apache/ignite-3/pull/6314#discussion_r2247706682
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutionContext.java:
##########
@@ -152,6 +156,7 @@ public ExecutionContext(
this.txAttributes = txAttributes;
this.timeZoneId = timeZoneId;
this.inBufSize = inBufSize < 0 ? Commons.IN_BUFFER_SIZE : inBufSize;
+ this.currentUser = username == null ? Commons.SYSTEM_USER_NAME : username;
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25588 Sql. Support CURRENT_USER function [ignite-3]
Posted by "AMashenkov (via GitHub)" <gi...@apache.org>.
AMashenkov merged PR #6314:
URL: https://github.com/apache/ignite-3/pull/6314
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25588 Sql. Support CURRENT_USER function [ignite-3]
Posted by "korlov42 (via GitHub)" <gi...@apache.org>.
korlov42 commented on code in PR #6314:
URL: https://github.com/apache/ignite-3/pull/6314#discussion_r2247220011
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutionContext.java:
##########
@@ -152,6 +156,7 @@ public ExecutionContext(
this.txAttributes = txAttributes;
this.timeZoneId = timeZoneId;
this.inBufSize = inBufSize < 0 ? Commons.IN_BUFFER_SIZE : inBufSize;
+ this.currentUser = username == null ? Commons.SYSTEM_USER_NAME : username;
Review Comment:
I think, the user must be known by this point, hence from execution point of view it never null. The better place to make decision is somewhere prior to call to queryExecutor. We might expose information about user in SQL_QUERIES view, and user must be unambiguously resolved to avoid discrepancies
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25588 Sql. Support CURRENT_USER function [ignite-3]
Posted by "zstan (via GitHub)" <gi...@apache.org>.
zstan commented on code in PR #6314:
URL: https://github.com/apache/ignite-3/pull/6314#discussion_r2241688940
##########
modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItSqlOperatorsTest.java:
##########
@@ -400,6 +403,22 @@ public void testCurrentTimeFunctions() {
assertExpression("LOCALTIMESTAMP").check();
}
+ @Test
+ public void testCurrentUser() {
+ assertExpression("CURRENT_USER")
+ .returns(Commons.SYSTEM_USER_NAME)
+ .columnMetadata(new MetadataMatcher().type(ColumnType.STRING))
+ .check();
+
+ sql("CREATE TABLE t1 (id INT PRIMARY KEY, val VARCHAR)");
+ sql("INSERT INTO t1 (id, val) VALUES (1, CURRENT_USER)");
+
+ assertQuery("SELECT val FROM t1 WHERE val = CURRENT_USER")
+ .returns(Commons.SYSTEM_USER_NAME)
+ .columnMetadata(new MetadataMatcher().type(ColumnType.STRING))
+ .check();
Review Comment:
seems this part is not related to Operators at all and duplicated in existing tests, or i miss smth ?
##########
modules/client-handler/src/main/java/org/apache/ignite/client/handler/JdbcConnectionContext.java:
##########
@@ -45,22 +45,30 @@ class JdbcConnectionContext {
private final ZoneId timeZoneId;
+ private final String userName;
+
private final ConcurrentMap<Long, CancelHandle> cancelHandles = new ConcurrentHashMap<>();
private @Nullable TxWithTimeTracker txWithTimeTracker;
JdbcConnectionContext(
TxManager txManager,
- ZoneId timeZoneId
+ ZoneId timeZoneId,
+ String userName
Review Comment:
```suggestion
@Nullable String userName
```
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/api/IgniteSqlImpl.java:
##########
@@ -660,7 +661,8 @@ private static SqlProperties toPropertiesBuilder(Statement statement) {
return new SqlProperties()
.timeZoneId(statement.timeZoneId())
.defaultSchema(IgniteNameUtils.parseIdentifier(statement.defaultSchema()))
- .queryTimeout(statement.queryTimeout(TimeUnit.MILLISECONDS));
+ .queryTimeout(statement.queryTimeout(TimeUnit.MILLISECONDS))
+ .userName(Commons.SYSTEM_USER_NAME);
Review Comment:
is it possible to move it into SqlProperties class with common default params initialization ? i.e.:
```
public class SqlProperties {
private String defaultSchema = SqlCommon.DEFAULT_SCHEMA_NAME;
...
private @Nullable String userName = Commons.SYSTEM_USER_NAME; ??
```
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/SqlOperationContext.java:
##########
@@ -104,6 +107,10 @@ public ZoneId timeZoneId() {
return timeZoneId;
}
+ public @Nullable String userName() {
Review Comment:
javadoc is still helpful
##########
modules/client-handler/src/main/java/org/apache/ignite/client/handler/JdbcConnectionContext.java:
##########
@@ -45,22 +45,30 @@ class JdbcConnectionContext {
private final ZoneId timeZoneId;
+ private final String userName;
+
private final ConcurrentMap<Long, CancelHandle> cancelHandles = new ConcurrentHashMap<>();
private @Nullable TxWithTimeTracker txWithTimeTracker;
JdbcConnectionContext(
TxManager txManager,
- ZoneId timeZoneId
+ ZoneId timeZoneId,
+ String userName
) {
this.txManager = txManager;
this.timeZoneId = timeZoneId;
+ this.userName = userName;
}
ZoneId timeZoneId() {
return timeZoneId;
}
+ String userName() {
Review Comment:
```suggestion
@Nullable String userName() {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25588 Sql. Support CURRENT_USER function [ignite-3]
Posted by "korlov42 (via GitHub)" <gi...@apache.org>.
korlov42 commented on code in PR #6314:
URL: https://github.com/apache/ignite-3/pull/6314#discussion_r2239560912
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/fsm/QueryExecutor.java:
##########
@@ -176,6 +177,10 @@ public CompletableFuture<AsyncSqlCursor<InternalSqlRow>> executeQuery(
@Nullable CancellationToken cancellationToken,
Object... params
) {
+ if (properties.userName() == null) {
+ properties.userName(Commons.SYSTEM_USER_NAME);
Review Comment:
`SqlProperties` is an input parameter of `org.apache.ignite.internal.sql.engine.QueryProcessor#queryAsync`. It's 1) supposed to be reusable and 2) not thread safe. We must not modify this inside query engine
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/SqlOperationContext.java:
##########
@@ -104,6 +107,10 @@ public ZoneId timeZoneId() {
return timeZoneId;
}
+ public @Nullable String userName() {
Review Comment:
please add the javadoc describing what does this parameter mean and when it expected to be null
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org