Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DB specific length support in JSONs/Varchars/binary data #3107

Merged
merged 15 commits into from
Jun 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,16 @@ public class DbPlatformType implements ExtraDbTypes {
*/
private final boolean canHaveLength;

/**
* The maximum length supported by this platform type. If length is too big, fallback is used.
*/
private final int maxLength;

/**
* Use this platform type if length exceedes
*/
private final DbPlatformType fallback;

/**
* Parse a type definition into a DbPlatformType.
* <p>
Expand All @@ -53,6 +63,41 @@ public DbPlatformType(String name, int defaultLength) {
this(name, defaultLength, 0);
}

/**
* Construct without length, but with a max length limit and a fallback type, that is used if maxLength is exceeded.
* This can be used to use <ul>
* <li>"longtext" for unspecified length</li>
* <li>"text" for length up to 2^16-1</li>
* <li>"mediumtext" for length up to 2^24-1</li>
* <li>"longtext" else</li>
* </ul>
*/
public DbPlatformType(String name, int maxLength, DbPlatformType fallback) {
this.name = name;
this.defaultLength = 0;
this.defaultScale = 0;
this.canHaveLength = false;
this.maxLength = maxLength;
this.fallback = fallback;
}

/**
* Construct with a given default length, a max length limit and a fallback type, that is used if maxLength is exceeded.
* This can be used to use <ul>
* <li>"varchar(255)" for unspecified length</li>
* <li>"varchar(N)" for N <= maxLength</li>
* <li>"varchar(max)" else</li>
* </ul>
*/
public DbPlatformType(String name, int defaultPrecision, int maxLength, DbPlatformType fallback) {
this.name = name;
this.defaultLength = defaultPrecision;
this.defaultScale = 0;
this.canHaveLength = true;
this.maxLength = maxLength;
this.fallback = fallback;
}

/**
* Construct for Decimal with precision and scale.
*/
Expand All @@ -61,6 +106,8 @@ public DbPlatformType(String name, int defaultPrecision, int defaultScale) {
this.defaultLength = defaultPrecision;
this.defaultScale = defaultScale;
this.canHaveLength = true;
this.maxLength = Integer.MAX_VALUE;
this.fallback = null;
}

/**
Expand All @@ -74,6 +121,8 @@ public DbPlatformType(String name, boolean canHaveLength) {
this.defaultLength = 0;
this.defaultScale = 0;
this.canHaveLength = canHaveLength;
this.maxLength = Integer.MAX_VALUE;
this.fallback = null;
}

/**
Expand Down Expand Up @@ -124,33 +173,33 @@ public String renderType(int deployLength, int deployScale) {
*/
public String renderType(int deployLength, int deployScale, boolean strict) {

StringBuilder sb = new StringBuilder();
sb.append(name);
if (canHaveLength || !strict) {
renderLengthScale(deployLength, deployScale, sb);
}
int len = deployLength != 0 ? deployLength : defaultLength;
if (len > maxLength) {
return fallback.renderType(deployLength, deployScale, strict);
} else {
StringBuilder sb = new StringBuilder();
sb.append(name);
if ((canHaveLength || !strict) && len > 0) {
renderLengthScale(len, deployScale, sb);
}

return sb.toString();
return sb.toString();
}
}

/**
* Render the length and scale part of the column definition.
*/
protected void renderLengthScale(int deployLength, int deployScale, StringBuilder sb) {
protected void renderLengthScale(int len, int deployScale, StringBuilder sb) {
// see if there is a precision/scale to add (or not)
int len = deployLength != 0 ? deployLength : defaultLength;
if (len == Integer.MAX_VALUE) {
sb.append("(max)"); // TODO: this is sqlserver specific
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 can remove the sqlserver workaround now

} else if (len > 0) {
sb.append("(");
sb.append(len);
int scale = deployScale != 0 ? deployScale : defaultScale;
if (scale > 0) {
sb.append(",");
sb.append(scale);
}
sb.append(")");
sb.append('(');
sb.append(len);
int scale = deployScale != 0 ? deployScale : defaultScale;
if (scale > 0) {
sb.append(',');
sb.append(scale);
}
sb.append(')');
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ class ClickHouseDbArray {
mapping.put("integer[]", "Array(UInt32)");
mapping.put("bigint[]", "Array(UInt64)");
mapping.put("float[]", "Array(Float32)");
mapping.put("float4[]", "Array(Float32)");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There were some issues for Clickhouse. They are in commit 81443b7

mapping.put("decimal[]", "Array(Decimal)");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,13 @@ public String convert(String type) {
if (type == null) {
return null;
}
type = extract(type);
String[] tmp = type.split(";", -1); // do not discard trailing empty strings
int index = extract(tmp);
if (index < tmp.length - 1) {
// this is a platform specific definition. So do not apply further conversions
return tmp[index];
}
type = tmp[index];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This detects, if there is a platform specific definition, e.g. @Column(columnDefinition = "mariadb,mysql;varchar(16000);") This definition is taken "as is" and not converted

if (type.contains("[]")) {
return convertArrayType(type);
}
Expand All @@ -294,18 +300,22 @@ protected String extract(String type) {
return null;
}
String[] tmp = type.split(";", -1); // do not discard trailing empty strings
if (tmp.length % 2 == 0) {
throw new IllegalArgumentException("You need an odd number of arguments in '" + type + "'. See Issue #2559 for details");
return tmp[extract(tmp)]; // else
}

protected int extract(String[] types) {
if (types.length % 2 == 0) {
throw new IllegalArgumentException("You need an odd number of arguments in '" + String.join(";", types) + "'. See Issue #2559 for details");
}
for (int i = 0; i < tmp.length - 2; i += 2) {
String[] platforms = tmp[i].split(",");
for (int i = 0; i < types.length - 2; i += 2) {
String[] platforms = types[i].split(",");
for (String plat : platforms) {
if (platform.isPlatform(Platform.valueOf(plat.toUpperCase(Locale.ENGLISH)))) {
return tmp[i + 1];
return i + 1;
}
}
}
return tmp[tmp.length - 1]; // else
return types.length - 1; // else
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,23 @@
public class MySqlPlatformTest {

MySqlPlatform mySqlPlatform = new MySqlPlatform();

private static int X = 0xFFFFFF;
@Test
public void testTypeConversion() {
PlatformDdl ddl = PlatformDdlBuilder.create(mySqlPlatform);
assertThat(ddl.convert("clob")).isEqualTo("longtext");
assertThat(ddl.convert("clob(65535)")).isEqualTo("text");
assertThat(ddl.convert("clob(65536)")).isEqualTo("mediumtext");
assertThat(ddl.convert("clob(16777215)")).isEqualTo("mediumtext");
assertThat(ddl.convert("clob(16777216)")).isEqualTo("longtext");
assertThat(ddl.convert("json")).isEqualTo("json");
assertThat(ddl.convert("jsonb")).isEqualTo("json");
assertThat(ddl.convert("varchar(20)")).isEqualTo("varchar(20)");
assertThat(ddl.convert("boolean")).isEqualTo("tinyint(1)");
assertThat(ddl.convert("bit")).isEqualTo("tinyint(1)");
assertThat(ddl.convert("decimal")).isEqualTo("decimal(16,3)");


}

}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public void testTypeConversion() {
PlatformDdl ddl = PlatformDdlBuilder.create(platform);

assertThat(ddl.convert("clob")).isEqualTo("nvarchar(max)");
assertThat(ddl.convert("blob")).isEqualTo("image");
assertThat(ddl.convert("blob")).isEqualTo("varbinary(max)");
assertThat(ddl.convert("json")).isEqualTo("nvarchar(max)");
assertThat(ddl.convert("jsonb")).isEqualTo("nvarchar(max)");

Expand All @@ -32,6 +32,10 @@ public void testTypeConversion() {
assertThat(ddl.convert("bit")).isEqualTo("bit");
assertThat(ddl.convert("tinyint")).isEqualTo("smallint");
assertThat(ddl.convert("binary(16)")).isEqualTo("binary(16)");

assertThat(ddl.convert("varchar")).isEqualTo("nvarchar(255)");
assertThat(ddl.convert("varchar(4000)")).isEqualTo("nvarchar(4000)");
assertThat(ddl.convert("varchar(4001)")).isEqualTo("nvarchar(max)");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public Properties setup(Config config) {
config.setUsername("default");
config.setPassword("");
config.setUrl("jdbc:clickhouse://${host}:${port}/${databaseName}");
config.setDriver("ru.yandex.clickhouse.ClickHouseDriver");
config.setDriver("com.clickhouse.jdbc.ClickHouseDriver");
config.datasourceDefaults();

return dockerProperties(config);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ void testLookupRender_given_sqlserver17() {
assertThat(dbTypeMap.lookup("json", false).renderType(0, 0)).isEqualTo("nvarchar(max)");
assertThat(dbTypeMap.lookup("jsonb", false).renderType(0, 0)).isEqualTo("nvarchar(max)");
assertThat(dbTypeMap.lookup("jsonclob", false).renderType(0, 0)).isEqualTo("nvarchar(max)");
assertThat(dbTypeMap.lookup("jsonblob", false).renderType(0, 0)).isEqualTo("image");
assertThat(dbTypeMap.lookup("jsonblob", false).renderType(0, 0)).isEqualTo("varbinary(max)");
assertThat(dbTypeMap.lookup("jsonvarchar", false).renderType(200, 0)).isEqualTo("nvarchar(200)");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ public void testRunMigration() throws IOException, SQLException {
"migtest_ckey_detail",
"migtest_ckey_parent",
"migtest_e_basic",
"migtest_e_test_binary",
"migtest_e_test_varchar",
"migtest_e_test_lob",
"migtest_e_test_json",
"migtest_e_enum",
"migtest_e_history",
"migtest_e_history2",
Expand Down
72 changes: 72 additions & 0 deletions ebean-test/src/test/java/misc/migration/v1_1/ETestBinary.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package misc.migration.v1_1;

import javax.persistence.Entity;
import javax.persistence.Id;
import javax.persistence.Table;
import javax.validation.constraints.Size;

@Entity
@Table(name = "migtest_e_test_binary")
public class ETestBinary {

@Id
int id;

@Size(max = 16)
byte[] test_byte16;

@Size(max = 256)
byte[] test_byte256;

@Size(max = 512)
byte[] test_byte512;

@Size(max = 1024)
byte[] test_byte1k;

@Size(max = 2 * 1024)
byte[] test_byte2k;

@Size(max = 4 * 1024)
byte[] test_byte4k;

@Size(max = 8 * 1024)
byte[] test_byte8k;

@Size(max = 16 * 1024)
byte[] test_byte16k;

@Size(max = 32 * 1024)
byte[] test_byte32k;

@Size(max = 64 * 1024)
byte[] test_byte64k;

@Size(max = 128 * 1024)
byte[] test_byte128k;

@Size(max = 256 * 1024)
byte[] test_byte256k;

@Size(max = 512 * 1024)
byte[] test_byte512k;

@Size(max = 1024 * 1024)
byte[] test_byte1m;

@Size(max = 2 * 1024 * 1024)
byte[] test_byte2m;

@Size(max = 4 * 1024 * 1024)
byte[] test_byte4m;

@Size(max = 8 * 1024 * 1024)
byte[] test_byte8m;

@Size(max = 16 * 1024 * 1024)
byte[] test_byte16m;

@Size(max = 32 * 1024 * 1024)
byte[] test_byte32m;

}
Loading