Skip to content

Commit 4a436b3

Browse files
Zxc4wonderZxc4wonder
authored
Merge pull request #2 from Zxc4wonder/fix/pg-alter-column-type
fix(postgresql): preserve ALTER COLUMN TYPE clauses
2 parents 33824c3 + a9e607e commit 4a436b3

14 files changed

Lines changed: 774 additions & 513 deletions

File tree

‎.github/workflows/publish-druid.yml‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
name: Publish Druid Package
2+
3+
on:
4+
workflow_dispatch:
5+
6+
permissions:
7+
contents: read
8+
packages: write
9+
10+
jobs:
11+
publish:
12+
runs-on: ubuntu-latest
13+
14+
steps:
15+
- name: Checkout source
16+
uses: actions/checkout@v6
17+
18+
- name: Set up Java and Maven
19+
uses: actions/setup-java@v4
20+
with:
21+
distribution: temurin
22+
java-version: "8"
23+
cache: maven
24+
server-id: github
25+
server-username: GITHUB_ACTOR
26+
server-password: GITHUB_TOKEN
27+
28+
- name: Verify Maven coordinates
29+
shell: bash
30+
run: |
31+
set -euo pipefail
32+
33+
GROUP_ID="$(
34+
mvn -pl core help:evaluate \
35+
-Dexpression=project.groupId \
36+
-q \
37+
-DforceStdout
38+
)"
39+
40+
ARTIFACT_ID="$(
41+
mvn -pl core help:evaluate \
42+
-Dexpression=project.artifactId \
43+
-q \
44+
-DforceStdout
45+
)"
46+
47+
VERSION="$(
48+
mvn -pl core help:evaluate \
49+
-Dexpression=project.version \
50+
-q \
51+
-DforceStdout
52+
)"
53+
54+
echo "Publishing: ${GROUP_ID}:${ARTIFACT_ID}:${VERSION}"
55+
56+
test "${GROUP_ID}" = "com.jumpserver"
57+
test "${ARTIFACT_ID}" = "druid"
58+
test "${VERSION}" = "1.2.28-jms.1"
59+
60+
- name: Publish package
61+
env:
62+
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
63+
run: |
64+
mvn \
65+
--batch-mode \
66+
--no-transfer-progress \
67+
-pl core \
68+
-am \
69+
clean deploy \
70+
-DskipTests \
71+
-Dcheckstyle.skip=true \
72+
-Dgpg.skip=true

‎core/pom.xml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
33
<modelVersion>4.0.0</modelVersion>
44
<parent>
5-
<groupId>com.alibaba</groupId>
5+
<groupId>com.jumpserver</groupId>
66
<artifactId>druid-parent</artifactId>
7-
<version>1.2.28</version>
7+
<version>1.2.28-jms.1</version>
88
<relativePath>../pom.xml</relativePath>
99
</parent>
1010
<artifactId>druid</artifactId>

‎core/src/main/java/com/alibaba/druid/sql/ast/statement/SQLAlterTableAlterColumn.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,16 +33,30 @@ public class SQLAlterTableAlterColumn extends SQLObjectImpl implements SQLAlterT
3333
private SQLName after;
3434
private SQLDataType dataType;
3535
private boolean toFirst;
36+
// PostgreSQL: ALTER COLUMN col TYPE <type> USING <expr>
37+
private SQLExpr using;
3638

3739
@Override
3840
protected void accept0(SQLASTVisitor visitor) {
3941
if (visitor.visit(this)) {
4042
acceptChild(visitor, column);
4143
acceptChild(visitor, setDefault);
44+
acceptChild(visitor, using);
4245
}
4346
visitor.endVisit(this);
4447
}
4548

49+
public SQLExpr getUsing() {
50+
return using;
51+
}
52+
53+
public void setUsing(SQLExpr using) {
54+
if (using != null) {
55+
using.setParent(this);
56+
}
57+
this.using = using;
58+
}
59+
4660
public SQLColumnDefinition getColumn() {
4761
return column;
4862
}

‎core/src/main/java/com/alibaba/druid/sql/dialect/postgresql/parser/PGSQLStatementParser.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -787,11 +787,24 @@ protected SQLAlterTableAlterColumn parseAlterColumn() {
787787
lexer.nextToken();
788788
accept(Token.NULL);
789789
alterColumn.setSetNotNull(true);
790+
} else if (lexer.identifierEquals("DATA")) {
791+
// ALTER COLUMN col SET DATA TYPE <type>
792+
lexer.nextToken();
793+
if (lexer.token() == Token.TYPE) {
794+
lexer.nextToken();
795+
} else {
796+
acceptIdentifier("TYPE");
797+
}
798+
alterColumn.setDataType(this.exprParser.parseDataType());
790799
} else {
791800
accept(Token.DEFAULT);
792801
SQLExpr defaultValue = this.exprParser.expr();
793802
alterColumn.setSetDefault(defaultValue);
794803
}
804+
} else if (lexer.token() == Token.TYPE || lexer.identifierEquals(FnvHash.Constants.TYPE)) {
805+
// ALTER COLUMN col TYPE <type>
806+
lexer.nextToken();
807+
alterColumn.setDataType(this.exprParser.parseDataType());
795808
} else if (lexer.token() == Token.DROP) {
796809
lexer.nextToken();
797810
if (lexer.token() == Token.NOT) {
@@ -804,6 +817,15 @@ protected SQLAlterTableAlterColumn parseAlterColumn() {
804817
}
805818
}
806819
}
820+
821+
// PostgreSQL: ALTER COLUMN col [SET DATA] TYPE <type> USING <expr>.
822+
// parseColumn may already have consumed "TYPE <type>" into the column definition, so this
823+
// check is outside the block above (issue #6064).
824+
if (lexer.token() == Token.USING || lexer.identifierEquals("USING")) {
825+
lexer.nextToken();
826+
alterColumn.setUsing(this.exprParser.expr());
827+
}
828+
807829
return alterColumn;
808830
}
809831

‎core/src/main/java/com/alibaba/druid/sql/dialect/postgresql/visitor/PGOutputVisitor.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,20 @@ public boolean visit(PGTypeCastExpr x) {
441441
return false;
442442
}
443443

444+
@Override
445+
public boolean visit(SQLCharacterDataType x) {
446+
super.visit(x);
447+
// PostgreSQL stores the COLLATE clause on the character data type (e.g. text COLLATE "x"
448+
// produced by a cast like (a)::text COLLATE "x", or a column definition); the generic
449+
// data type output drops it, so render it here (see issue #6573).
450+
String collate = x.getCollate();
451+
if (collate != null) {
452+
print0(ucase ? " COLLATE " : " collate ");
453+
print0(collate);
454+
}
455+
return false;
456+
}
457+
444458
@Override
445459
public boolean visit(PGExtractExpr x) {
446460
print0(ucase ? "EXTRACT(" : "extract(");

‎core/src/main/java/com/alibaba/druid/sql/visitor/SQLASTOutputVisitor.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5949,6 +5949,11 @@ public boolean visit(SQLAlterTableAlterColumn x) {
59495949
dataType.accept(this);
59505950
}
59515951

5952+
if (x.getUsing() != null) { // postgresql: ALTER COLUMN col TYPE <type> USING <expr>
5953+
print0(ucase ? " USING " : " using ");
5954+
x.getUsing().accept(this);
5955+
}
5956+
59525957
final SQLName after = x.getAfter();
59535958
if (after != null) {
59545959
print0(ucase ? " AFTER " : " after ");

‎core/src/test/java/com/alibaba/druid/bvt/sql/postgresql/issues/JumpServer16341Test.java‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
/*
2+
* Copyright 1999-2017 Alibaba Group Holding Ltd.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.alibaba.druid.bvt.sql.postgresql.issues;
17+
18+
import com.alibaba.druid.DbType;
19+
import com.alibaba.druid.sql.SQLUtils;
20+
import com.alibaba.druid.sql.ast.SQLStatement;
21+
import org.junit.Test;
22+
23+
import java.util.List;
24+
25+
import static org.junit.Assert.assertEquals;
26+
import static org.junit.Assert.assertTrue;
27+
28+
/**
29+
* Regression test for JumpServer issue #16341.
30+
*
31+
* <p>Druid 1.2.28 fails to round-trip several PostgreSQL {@code ALTER COLUMN} forms:
32+
* <ul>
33+
* <li>{@code ALTER COLUMN ... TYPE <type> COLLATE <collation>} drops the COLLATE clause;</li>
34+
* <li>{@code ALTER COLUMN ... SET DATA TYPE <type>} throws a ParserException
35+
* (the SET branch only accepted NOT NULL / DEFAULT);</li>
36+
* <li>{@code ALTER COLUMN ... TYPE <type> USING <expr>} throws a ParserException
37+
* at the USING keyword.</li>
38+
* </ul>
39+
*
40+
* <p>Covered scenarios:
41+
* <ol>
42+
* <li>{@code TYPE} + {@code COLLATE};</li>
43+
* <li>{@code TYPE CHARACTER VARYING(n)};</li>
44+
* <li>{@code SET DATA TYPE};</li>
45+
* <li>{@code TYPE ... USING}.</li>
46+
* </ol>
47+
*
48+
* <p>The fix is a targeted backport of upstream commits
49+
* {@code d42545ff} (PG {@code SET DATA TYPE} / {@code COLLATE}, issues #6467 / #6573) and
50+
* {@code 26b856b1} (PG {@code TYPE ... USING}, issue #6064).
51+
*/
52+
public class JumpServer16341Test {
53+
private static String roundTrip(String sql) {
54+
List<SQLStatement> statements = SQLUtils.parseStatements(sql, DbType.postgresql);
55+
assertEquals(1, statements.size());
56+
return SQLUtils.toSQLString(statements.get(0), DbType.postgresql);
57+
}
58+
59+
/** Collapse all runs of whitespace into single spaces, for format-independent checks. */
60+
private static String squash(String s) {
61+
return s.replaceAll("\\s+", " ").trim();
62+
}
63+
64+
/**
65+
* Scenario 1: ALTER COLUMN ... TYPE <char type> COLLATE <collation>.
66+
*
67+
* <p>The TYPE keyword and the COLLATE clause (with its quoted schema/name) must both survive
68+
* the round-trip; the output must not degrade to a bare {@code ALTER COLUMN col <type>} form.
69+
*/
70+
@Test
71+
public void scenario1_type_and_collate() {
72+
String sql = "ALTER TABLE \"public\".\"test_alter_table\" "
73+
+ "ALTER COLUMN \"job_code\" "
74+
+ "TYPE VARCHAR(255) COLLATE \"pg_catalog\".\"default\"";
75+
76+
String out = roundTrip(sql);
77+
String upper = squash(out).toUpperCase();
78+
79+
// Type-change keyword (TYPE or SET DATA TYPE) must remain before the data type.
80+
assertTrue("output must keep TYPE/SET DATA TYPE before the data type: " + out,
81+
upper.contains("TYPE VARCHAR(255)"));
82+
83+
// COLLATE keyword and its quoted content must be preserved exactly.
84+
assertTrue("COLLATE keyword must be present: " + out, upper.contains("COLLATE"));
85+
assertTrue("COLLATE content must be preserved: " + out,
86+
out.contains("\"pg_catalog\".\"default\""));
87+
}
88+
89+
/**
90+
* Scenario 2: ALTER COLUMN ... TYPE CHARACTER VARYING(n).
91+
*
92+
* <p>The TYPE keyword and the {@code CHARACTER VARYING(n)} type name must be preserved.
93+
*/
94+
@Test
95+
public void scenario2_type_character_varying() {
96+
String sql = "ALTER TABLE \"public\".\"test_alter_table\" "
97+
+ "ALTER COLUMN \"job_code\" "
98+
+ "TYPE CHARACTER VARYING(255)";
99+
100+
String out = roundTrip(sql);
101+
String upper = squash(out).toUpperCase();
102+
103+
assertTrue("output must keep TYPE/SET DATA TYPE: " + out, upper.contains("TYPE"));
104+
assertTrue("CHARACTER VARYING(255) must be preserved: " + out,
105+
upper.contains("CHARACTER VARYING(255)"));
106+
}
107+
108+
/**
109+
* Scenario 3: ALTER COLUMN ... SET DATA TYPE <type>.
110+
*
111+
* <p>Must parse without error and re-emit a legal type-change statement (SET DATA TYPE or
112+
* normalized to TYPE).
113+
*/
114+
@Test
115+
public void scenario3_set_data_type() {
116+
String sql = "ALTER TABLE test_alter_table "
117+
+ "ALTER COLUMN job_code "
118+
+ "SET DATA TYPE VARCHAR(255)";
119+
120+
String out = roundTrip(sql);
121+
String upper = squash(out).toUpperCase();
122+
123+
assertTrue("output must keep TYPE/SET DATA TYPE and VARCHAR(255): " + out,
124+
upper.contains("TYPE") && upper.contains("VARCHAR(255)"));
125+
}
126+
127+
/**
128+
* Scenario 4: ALTER COLUMN ... TYPE <type> USING <expr>.
129+
*
130+
* <p>The type-change keyword and the USING expression must both survive the round-trip.
131+
*/
132+
@Test
133+
public void scenario4_type_using() {
134+
String sql = "ALTER TABLE test_alter_table "
135+
+ "ALTER COLUMN value TYPE bigint USING value::bigint";
136+
137+
String out = roundTrip(sql);
138+
String upper = squash(out).toUpperCase();
139+
140+
assertTrue("output must keep TYPE/SET DATA TYPE bigint: " + out,
141+
upper.contains("TYPE") && upper.contains("BIGINT"));
142+
assertTrue("USING keyword must be present: " + out, upper.contains("USING"));
143+
// The USING expression must be preserved (case-insensitive on the bare identifiers,
144+
// but the :: cast form and structure must remain).
145+
assertTrue("USING expression must be preserved: " + out,
146+
out.toLowerCase().contains("value::bigint"));
147+
}
148+
}

‎core/src/test/resources/bvt/parser/postgresql/17.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -369,8 +369,8 @@ CREATE TABLE "public"."city" (
369369
) WITH (OIDS=FALSE);
370370
--------------------
371371
CREATE TABLE "public"."city" (
372-
"id" varchar(6) NOT NULL,
373-
"name" varchar(32) NOT NULL
372+
"id" varchar(6) COLLATE "default" NOT NULL,
373+
"name" varchar(32) COLLATE "default" NOT NULL
374374
)
375375
WITH (
376376
OIDS = false

‎druid-demo-petclinic/pom.xml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,9 @@
3232

3333
<dependencies>
3434
<dependency>
35-
<groupId>com.alibaba</groupId>
35+
<groupId>com.jumpserver</groupId>
3636
<artifactId>druid-spring-boot-starter</artifactId>
37-
<version>1.2.28</version>
37+
<version>1.2.28-jms.1</version>
3838
</dependency>
3939
<!-- Spring and Spring Boot dependencies -->
4040
<dependency>

‎druid-spring-boot-3-starter/pom.xml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@
33
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
44
<modelVersion>4.0.0</modelVersion>
55
<parent>
6-
<groupId>com.alibaba</groupId>
6+
<groupId>com.jumpserver</groupId>
77
<artifactId>druid-parent</artifactId>
8-
<version>1.2.28</version>
8+
<version>1.2.28-jms.1</version>
99
<relativePath>../pom.xml</relativePath>
1010
</parent>
1111
<artifactId>druid-spring-boot-3-starter</artifactId>
@@ -38,7 +38,7 @@
3838

3939
<dependencies>
4040
<dependency>
41-
<groupId>com.alibaba</groupId>
41+
<groupId>com.jumpserver</groupId>
4242
<artifactId>druid</artifactId>
4343
<version>${project.version}</version>
4444
</dependency>

0 commit comments

Comments
 (0)