This is an automated email from the ASF dual-hosted git repository.
zhangliang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shardingsphere.git
The following commit(s) were added to refs/heads/master by this push:
new cb1652a866c proxy, parser: honor character_set_results for SET NAMES
(#38186)
cb1652a866c is described below
commit cb1652a866ce8296010bee537d0902816619bb0a
Author: Aviral Garg <[email protected]>
AuthorDate: Fri Feb 27 15:26:57 2026 +0530
proxy, parser: honor character_set_results for SET NAMES (#38186)
* proxy, parser: honor character_set_results for SET NAMES
* proxy: skip charset update for character_set_results NULL
---
.../statement/type/DorisDALStatementVisitor.java | 30 ++++++++++++++++-----
.../statement/type/MySQLDALStatementVisitor.java | 30 ++++++++++++++++-----
.../variable/charset/CharsetSetExecutor.java | 3 ++-
.../variable/charset/CharsetVariableProvider.java | 11 ++++++++
.../variable/charset/CharsetSetExecutorTest.java | 31 ++++++++++++++++++++++
.../charset/MySQLCharsetVariableProvider.java | 7 ++++-
.../MySQLSetVariableAdminExecutorTest.java | 24 +++++++++++++++++
.../charset/MySQLCharsetVariableProviderTest.java | 16 +++++++++--
test/it/parser/src/main/resources/case/dal/set.xml | 12 +++++++++
9 files changed, 148 insertions(+), 16 deletions(-)
diff --git
a/parser/sql/engine/dialect/doris/src/main/java/org/apache/shardingsphere/sql/parser/engine/doris/visitor/statement/type/DorisDALStatementVisitor.java
b/parser/sql/engine/dialect/doris/src/main/java/org/apache/shardingsphere/sql/parser/engine/doris/visitor/statement/type/DorisDALStatementVisitor.java
index 63275446874..e4356ab16d4 100644
---
a/parser/sql/engine/dialect/doris/src/main/java/org/apache/shardingsphere/sql/parser/engine/doris/visitor/statement/type/DorisDALStatementVisitor.java
+++
b/parser/sql/engine/dialect/doris/src/main/java/org/apache/shardingsphere/sql/parser/engine/doris/visitor/statement/type/DorisDALStatementVisitor.java
@@ -945,10 +945,32 @@ public final class DorisDALStatementVisitor extends
DorisStatementVisitor implem
private List<VariableAssignSegment> getVariableAssigns(final
OptionValueListContext ctx) {
List<VariableAssignSegment> result = new LinkedList<>();
- result.add(null == ctx.optionValueNoOptionType() ?
getVariableAssignSegment(ctx) :
getVariableAssignSegment(ctx.optionValueNoOptionType()));
+ if (null == ctx.optionValueNoOptionType()) {
+ result.add(getVariableAssignSegment(ctx));
+ } else {
+
result.addAll(getVariableAssignSegments(ctx.optionValueNoOptionType()));
+ }
for (OptionValueContext each : ctx.optionValue()) {
- result.add(getVariableAssignSegment(each));
+ if (null == each.optionValueNoOptionType()) {
+ result.add(getVariableAssignSegment(each));
+ } else {
+
result.addAll(getVariableAssignSegments(each.optionValueNoOptionType()));
+ }
+ }
+ return result;
+ }
+
+ private List<VariableAssignSegment> getVariableAssignSegments(final
OptionValueNoOptionTypeContext ctx) {
+ if (null == ctx.NAMES()) {
+ return Collections.singletonList(getVariableAssignSegment(ctx));
}
+ int startIndex = ctx.start.getStartIndex();
+ int stopIndex = ctx.stop.getStopIndex();
+ String assignValue = ctx.charsetName().getText();
+ List<VariableAssignSegment> result = new LinkedList<>();
+ result.add(new VariableAssignSegment(startIndex, stopIndex, new
VariableSegment(startIndex, stopIndex, "character_set_client"), assignValue));
+ result.add(new VariableAssignSegment(startIndex, stopIndex, new
VariableSegment(startIndex, stopIndex, "character_set_results"), assignValue));
+ result.add(new VariableAssignSegment(startIndex, stopIndex, new
VariableSegment(startIndex, stopIndex, "character_set_connection"),
assignValue));
return result;
}
@@ -972,10 +994,6 @@ public final class DorisDALStatementVisitor extends
DorisStatementVisitor implem
}
private VariableSegment getVariableSegment(final
OptionValueNoOptionTypeContext ctx) {
- if (null != ctx.NAMES()) {
- // TODO Consider setting all three system variables:
character_set_client, character_set_results, character_set_connection
- return new
VariableSegment(ctx.NAMES().getSymbol().getStartIndex(),
ctx.NAMES().getSymbol().getStopIndex(), "character_set_client");
- }
if (null != ctx.internalVariableName()) {
return new
VariableSegment(ctx.internalVariableName().start.getStartIndex(),
ctx.internalVariableName().stop.getStopIndex(),
ctx.internalVariableName().getText());
}
diff --git
a/parser/sql/engine/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/engine/mysql/visitor/statement/type/MySQLDALStatementVisitor.java
b/parser/sql/engine/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/engine/mysql/visitor/statement/type/MySQLDALStatementVisitor.java
index ff8ee6e6f52..068d0a1991d 100644
---
a/parser/sql/engine/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/engine/mysql/visitor/statement/type/MySQLDALStatementVisitor.java
+++
b/parser/sql/engine/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/engine/mysql/visitor/statement/type/MySQLDALStatementVisitor.java
@@ -788,13 +788,35 @@ public final class MySQLDALStatementVisitor extends
MySQLStatementVisitor implem
private List<VariableAssignSegment> getVariableAssigns(final
OptionValueListContext ctx) {
List<VariableAssignSegment> result = new LinkedList<>();
- result.add(null == ctx.optionValueNoOptionType() ?
getVariableAssignSegment(ctx) :
getVariableAssignSegment(ctx.optionValueNoOptionType()));
+ if (null == ctx.optionValueNoOptionType()) {
+ result.add(getVariableAssignSegment(ctx));
+ } else {
+
result.addAll(getVariableAssignSegments(ctx.optionValueNoOptionType()));
+ }
for (OptionValueContext each : ctx.optionValue()) {
- result.add(getVariableAssignSegment(each));
+ if (null == each.optionValueNoOptionType()) {
+ result.add(getVariableAssignSegment(each));
+ } else {
+
result.addAll(getVariableAssignSegments(each.optionValueNoOptionType()));
+ }
}
return result;
}
+ private List<VariableAssignSegment> getVariableAssignSegments(final
OptionValueNoOptionTypeContext ctx) {
+ if (null == ctx.NAMES()) {
+ return Collections.singletonList(getVariableAssignSegment(ctx));
+ }
+ int startIndex = ctx.start.getStartIndex();
+ int stopIndex = ctx.stop.getStopIndex();
+ String assignValue = ctx.charsetName().getText();
+ List<VariableAssignSegment> result = new LinkedList<>();
+ result.add(new VariableAssignSegment(startIndex, stopIndex, new
VariableSegment(startIndex, stopIndex, "character_set_client"), assignValue));
+ result.add(new VariableAssignSegment(startIndex, stopIndex, new
VariableSegment(startIndex, stopIndex, "character_set_results"), assignValue));
+ result.add(new VariableAssignSegment(startIndex, stopIndex, new
VariableSegment(startIndex, stopIndex, "character_set_connection"),
assignValue));
+ return result;
+ }
+
private VariableAssignSegment getVariableAssignSegment(final
OptionValueContext ctx) {
if (null != ctx.optionValueNoOptionType()) {
return getVariableAssignSegment(ctx.optionValueNoOptionType());
@@ -815,10 +837,6 @@ public final class MySQLDALStatementVisitor extends
MySQLStatementVisitor implem
}
private VariableSegment getVariableSegment(final
OptionValueNoOptionTypeContext ctx) {
- if (null != ctx.NAMES()) {
- // TODO Consider setting all three system variables:
character_set_client, character_set_results, character_set_connection
- return new
VariableSegment(ctx.NAMES().getSymbol().getStartIndex(),
ctx.NAMES().getSymbol().getStopIndex(), "character_set_client");
- }
if (null != ctx.internalVariableName()) {
return new
VariableSegment(ctx.internalVariableName().start.getStartIndex(),
ctx.internalVariableName().stop.getStopIndex(),
ctx.internalVariableName().getText());
}
diff --git
a/proxy/backend/core/src/main/java/org/apache/shardingsphere/proxy/backend/handler/admin/executor/variable/charset/CharsetSetExecutor.java
b/proxy/backend/core/src/main/java/org/apache/shardingsphere/proxy/backend/handler/admin/executor/variable/charset/CharsetSetExecutor.java
index 416fc741513..f7779d0dabf 100644
---
a/proxy/backend/core/src/main/java/org/apache/shardingsphere/proxy/backend/handler/admin/executor/variable/charset/CharsetSetExecutor.java
+++
b/proxy/backend/core/src/main/java/org/apache/shardingsphere/proxy/backend/handler/admin/executor/variable/charset/CharsetSetExecutor.java
@@ -43,7 +43,8 @@ public final class CharsetSetExecutor {
*/
public void set(final String variableName, final String toBeAssignedValue)
{
Optional<CharsetVariableProvider> charsetVariableProvider =
DatabaseTypedSPILoader.findService(CharsetVariableProvider.class, databaseType);
- if (charsetVariableProvider.isPresent() &&
charsetVariableProvider.get().getCharsetVariables().stream().anyMatch(each ->
each.equalsIgnoreCase(variableName))) {
+ if (charsetVariableProvider.isPresent() &&
charsetVariableProvider.get().getCharsetVariables().stream().anyMatch(each ->
each.equalsIgnoreCase(variableName))
+ && charsetVariableProvider.get().shouldSet(variableName,
toBeAssignedValue)) {
connectionSession.getAttributeMap().attr(CommonConstants.CHARSET_ATTRIBUTE_KEY).set(charsetVariableProvider.get().parseCharset(toBeAssignedValue));
}
}
diff --git
a/proxy/backend/core/src/main/java/org/apache/shardingsphere/proxy/backend/handler/admin/executor/variable/charset/CharsetVariableProvider.java
b/proxy/backend/core/src/main/java/org/apache/shardingsphere/proxy/backend/handler/admin/executor/variable/charset/CharsetVariableProvider.java
index 5bbff920ff5..052ed079545 100644
---
a/proxy/backend/core/src/main/java/org/apache/shardingsphere/proxy/backend/handler/admin/executor/variable/charset/CharsetVariableProvider.java
+++
b/proxy/backend/core/src/main/java/org/apache/shardingsphere/proxy/backend/handler/admin/executor/variable/charset/CharsetVariableProvider.java
@@ -36,6 +36,17 @@ public interface CharsetVariableProvider extends
DatabaseTypedSPI {
*/
Collection<String> getCharsetVariables();
+ /**
+ * Determine whether charset should be set.
+ *
+ * @param variableName variable name
+ * @param variableValue variable value
+ * @return true if charset should be set
+ */
+ default boolean shouldSet(final String variableName, final String
variableValue) {
+ return true;
+ }
+
/**
* Parse charset from variable.
*
diff --git
a/proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/handler/admin/executor/variable/charset/CharsetSetExecutorTest.java
b/proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/handler/admin/executor/variable/charset/CharsetSetExecutorTest.java
index e7fdce73818..5bb4982060f 100644
---
a/proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/handler/admin/executor/variable/charset/CharsetSetExecutorTest.java
+++
b/proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/handler/admin/executor/variable/charset/CharsetSetExecutorTest.java
@@ -34,6 +34,7 @@ import java.util.Optional;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -48,6 +49,7 @@ class CharsetSetExecutorTest {
void assertSetWhenProviderPresentAndVariableMatched() {
CharsetVariableProvider provider = mock(CharsetVariableProvider.class);
when(provider.getCharsetVariables()).thenReturn(Collections.singletonList("character_set_client"));
+ when(provider.shouldSet("CHARACTER_SET_CLIENT",
"utf8")).thenReturn(true);
when(provider.parseCharset("utf8")).thenReturn(StandardCharsets.UTF_8);
when(DatabaseTypedSPILoader.findService(CharsetVariableProvider.class,
databaseType)).thenReturn(Optional.of(provider));
AttributeMap attributeMap = mock(AttributeMap.class);
@@ -72,4 +74,33 @@ class CharsetSetExecutorTest {
when(DatabaseTypedSPILoader.findService(CharsetVariableProvider.class,
databaseType)).thenReturn(Optional.of(provider));
assertDoesNotThrow(() -> new CharsetSetExecutor(databaseType,
mock()).set("character_set_client", "utf8"));
}
+
+ @SuppressWarnings({"rawtypes", "unchecked"})
+ @Test
+ void assertSetWhenCharacterSetResultsMatched() {
+ CharsetVariableProvider provider = mock(CharsetVariableProvider.class);
+
when(provider.getCharsetVariables()).thenReturn(Collections.singletonList("character_set_results"));
+ when(provider.shouldSet("CHARACTER_SET_RESULTS",
"utf8")).thenReturn(true);
+ when(provider.parseCharset("utf8")).thenReturn(StandardCharsets.UTF_8);
+ when(DatabaseTypedSPILoader.findService(CharsetVariableProvider.class,
databaseType)).thenReturn(Optional.of(provider));
+ AttributeMap attributeMap = mock(AttributeMap.class);
+ Attribute attribute = mock(Attribute.class);
+
when(attributeMap.attr(CommonConstants.CHARSET_ATTRIBUTE_KEY)).thenReturn(attribute);
+ ConnectionSession connectionSession = mock(ConnectionSession.class);
+ when(connectionSession.getAttributeMap()).thenReturn(attributeMap);
+ new CharsetSetExecutor(databaseType,
connectionSession).set("CHARACTER_SET_RESULTS", "utf8");
+ verify(attribute).set(StandardCharsets.UTF_8);
+ }
+
+ @SuppressWarnings({"rawtypes", "unchecked"})
+ @Test
+ void assertSetWhenProviderRejectsVariableValue() {
+ CharsetVariableProvider provider = mock(CharsetVariableProvider.class);
+
when(provider.getCharsetVariables()).thenReturn(Collections.singletonList("character_set_results"));
+ when(provider.shouldSet("CHARACTER_SET_RESULTS",
"NULL")).thenReturn(false);
+ when(DatabaseTypedSPILoader.findService(CharsetVariableProvider.class,
databaseType)).thenReturn(Optional.of(provider));
+ ConnectionSession connectionSession = mock(ConnectionSession.class);
+ new CharsetSetExecutor(databaseType,
connectionSession).set("CHARACTER_SET_RESULTS", "NULL");
+ verify(connectionSession, never()).getAttributeMap();
+ }
}
diff --git
a/proxy/backend/dialect/mysql/src/main/java/org/apache/shardingsphere/proxy/backend/mysql/handler/admin/executor/variable/charset/MySQLCharsetVariableProvider.java
b/proxy/backend/dialect/mysql/src/main/java/org/apache/shardingsphere/proxy/backend/mysql/handler/admin/executor/variable/charset/MySQLCharsetVariableProvider.java
index cd3344b6dca..0b364373a98 100644
---
a/proxy/backend/dialect/mysql/src/main/java/org/apache/shardingsphere/proxy/backend/mysql/handler/admin/executor/variable/charset/MySQLCharsetVariableProvider.java
+++
b/proxy/backend/dialect/mysql/src/main/java/org/apache/shardingsphere/proxy/backend/mysql/handler/admin/executor/variable/charset/MySQLCharsetVariableProvider.java
@@ -35,7 +35,12 @@ public final class MySQLCharsetVariableProvider implements
CharsetVariableProvid
@Override
public Collection<String> getCharsetVariables() {
- return Arrays.asList("charset", "character_set_client");
+ return Arrays.asList("charset", "character_set_client",
"character_set_results", "character_set_connection");
+ }
+
+ @Override
+ public boolean shouldSet(final String variableName, final String
variableValue) {
+ return !"character_set_results".equalsIgnoreCase(variableName) ||
!"null".equalsIgnoreCase(formatValue(variableValue));
}
@Override
diff --git
a/proxy/backend/dialect/mysql/src/test/java/org/apache/shardingsphere/proxy/backend/mysql/handler/admin/executor/MySQLSetVariableAdminExecutorTest.java
b/proxy/backend/dialect/mysql/src/test/java/org/apache/shardingsphere/proxy/backend/mysql/handler/admin/executor/MySQLSetVariableAdminExecutorTest.java
index 7c99e034ef9..00135383c3f 100644
---
a/proxy/backend/dialect/mysql/src/test/java/org/apache/shardingsphere/proxy/backend/mysql/handler/admin/executor/MySQLSetVariableAdminExecutorTest.java
+++
b/proxy/backend/dialect/mysql/src/test/java/org/apache/shardingsphere/proxy/backend/mysql/handler/admin/executor/MySQLSetVariableAdminExecutorTest.java
@@ -77,6 +77,30 @@ class MySQLSetVariableAdminExecutorTest {
assertThat(connectionSession.getAttributeMap().attr(CommonConstants.CHARSET_ATTRIBUTE_KEY).get(),
is(StandardCharsets.UTF_8));
}
+ @Test
+ void assertExecuteWithCharacterSetResults() throws SQLException {
+ SetStatement setStatement = new SetStatement(databaseType,
Collections.singletonList(
+ new VariableAssignSegment(0, 0, new VariableSegment(0, 0,
"character_set_results"), "'utf8mb4'")));
+ MySQLSetVariableAdminExecutor executor = new
MySQLSetVariableAdminExecutor(setStatement);
+ ConnectionSession connectionSession = mock(ConnectionSession.class);
+ when(connectionSession.getAttributeMap()).thenReturn(new
DefaultAttributeMap());
+ executor.execute(connectionSession, mock());
+
assertThat(connectionSession.getAttributeMap().attr(CommonConstants.CHARSET_ATTRIBUTE_KEY).get(),
is(StandardCharsets.UTF_8));
+ }
+
+ @Test
+ void assertExecuteWithCharacterSetResultsAsNull() throws SQLException {
+ SetStatement setStatement = new SetStatement(databaseType,
Collections.singletonList(
+ new VariableAssignSegment(0, 0, new VariableSegment(0, 0,
"character_set_results"), "NULL")));
+ MySQLSetVariableAdminExecutor executor = new
MySQLSetVariableAdminExecutor(setStatement);
+ ConnectionSession connectionSession = mock(ConnectionSession.class);
+ DefaultAttributeMap attributeMap = new DefaultAttributeMap();
+
attributeMap.attr(CommonConstants.CHARSET_ATTRIBUTE_KEY).set(StandardCharsets.UTF_8);
+ when(connectionSession.getAttributeMap()).thenReturn(attributeMap);
+ executor.execute(connectionSession, mock());
+
assertThat(connectionSession.getAttributeMap().attr(CommonConstants.CHARSET_ATTRIBUTE_KEY).get(),
is(StandardCharsets.UTF_8));
+ }
+
private ConnectionContext mockConnectionContext() {
ConnectionContext result = mock(ConnectionContext.class);
when(result.getCurrentDatabaseName()).thenReturn(Optional.of("foo_db"));
diff --git
a/proxy/backend/dialect/mysql/src/test/java/org/apache/shardingsphere/proxy/backend/mysql/handler/admin/executor/variable/charset/MySQLCharsetVariableProviderTest.java
b/proxy/backend/dialect/mysql/src/test/java/org/apache/shardingsphere/proxy/backend/mysql/handler/admin/executor/variable/charset/MySQLCharsetVariableProviderTest.java
index 1ab31b9bf02..919f452ed0d 100644
---
a/proxy/backend/dialect/mysql/src/test/java/org/apache/shardingsphere/proxy/backend/mysql/handler/admin/executor/variable/charset/MySQLCharsetVariableProviderTest.java
+++
b/proxy/backend/dialect/mysql/src/test/java/org/apache/shardingsphere/proxy/backend/mysql/handler/admin/executor/variable/charset/MySQLCharsetVariableProviderTest.java
@@ -35,6 +35,8 @@ import java.util.stream.Stream;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.assertThrows;
class MySQLCharsetVariableProviderTest {
@@ -45,7 +47,7 @@ class MySQLCharsetVariableProviderTest {
@Test
void assertCharsetVariablesAndDatabaseType() {
- assertThat(provider.getCharsetVariables(), contains("charset",
"character_set_client"));
+ assertThat(provider.getCharsetVariables(), contains("charset",
"character_set_client", "character_set_results", "character_set_connection"));
}
@Test
@@ -53,7 +55,17 @@ class MySQLCharsetVariableProviderTest {
assertThrows(UnknownCharsetException.class, () ->
provider.parseCharset("unknown_charset"));
}
- @ParameterizedTest
+ @Test
+ void assertShouldSetWhenCharacterSetResultsWithNull() {
+ assertFalse(provider.shouldSet("character_set_results", "NULL"));
+ }
+
+ @Test
+ void assertShouldSetWhenCharacterSetResultsWithCharset() {
+ assertTrue(provider.shouldSet("character_set_results", "'utf8mb4'"));
+ }
+
+ @ParameterizedTest(name = "{0}")
@MethodSource("successArguments")
void assertParseCharset(final String input, final Charset expected) {
assertThat(provider.parseCharset(input), is(expected));
diff --git a/test/it/parser/src/main/resources/case/dal/set.xml
b/test/it/parser/src/main/resources/case/dal/set.xml
index 0ef64b40479..61ba695d862 100644
--- a/test/it/parser/src/main/resources/case/dal/set.xml
+++ b/test/it/parser/src/main/resources/case/dal/set.xml
@@ -99,6 +99,12 @@
<parameter-assign value="'utf8'">
<parameter name="character_set_client" />
</parameter-assign>
+ <parameter-assign value="'utf8'">
+ <parameter name="character_set_results" />
+ </parameter-assign>
+ <parameter-assign value="'utf8'">
+ <parameter name="character_set_connection" />
+ </parameter-assign>
</set-parameter>
<set-parameter sql-case-id="set_character_set_doris">
<parameter-assign value="utf8mb4">
@@ -112,6 +118,12 @@
<parameter-assign value="'utf8'">
<parameter name="character_set_client" />
</parameter-assign>
+ <parameter-assign value="'utf8'">
+ <parameter name="character_set_results" />
+ </parameter-assign>
+ <parameter-assign value="'utf8'">
+ <parameter name="character_set_connection" />
+ </parameter-assign>
</set-parameter>
<set-parameter sql-case-id="set_charset_postgresql">
<parameter-assign value="'UTF8'">
