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

Creating tables using DSLContext.ddl() converts VARBINARY columns to TEXT in MySQL #9473

Closed
trdesilva opened this issue Oct 31, 2019 · 6 comments

Comments

@trdesilva
Copy link

trdesilva commented Oct 31, 2019

Behavior:

When creating tables with DSLContext.ddl(), columns with type SQLDataType.VARBINARY are instead created with type TEXT in the database. I expect the VARBINARY type to be preserved.

Repro:

See https://github.com/trdesilva/jOOQ-mcve

@Test
public void mcveTest() {
    ctx.ddl(Testtable.TESTTABLE, new DDLExportConfiguration().createTableIfNotExists(true)).executeBatch();
    byte[] bytes = new byte[256];
    for (int i = 0; i < bytes.length; i++) {
        bytes[i] = (byte)(i - Byte.MAX_VALUE);
    }
    // this throws with the following on MySQL 5.6.43, but not 5.6.39:
    // org.jooq.exception.DataAccessException: SQL [insert into `TestTable` (`Foo`, `Bar`) values (?, ?)]; Incorrect string value: '\x81\x82\x83\x84\x85\x86...' for column 'Foo' at row 1
    ctx.insertInto(Testtable.TESTTABLE).values(bytes, bytes).execute();
}

Versions:

  • jOOQ: 3.12.2
  • Java: OpenJDK 11.0.4+11
  • Database (include vendor): Oracle MySQL 5.6.39/5.6.43 Community Server
  • OS: Ubuntu 16.04/AWS RDS
  • JDBC Driver (include name if inofficial driver): mysql-connector-java 5.1.46
trdesilva added a commit to trdesilva/jOOQ-mcve that referenced this issue Oct 31, 2019
@lukaseder lukaseder added this to the Version 3.13.0 milestone Oct 31, 2019
@lukaseder lukaseder self-assigned this Oct 31, 2019
@lukaseder
Copy link
Member

Thank you very much for your report. This issue was introduced by #6745

It seems we're generating the TEXT type for all types that have DataType.hasLength(), but which do not have an actual length(), which is incorrect, of course.

This can be demonstrated with:

public static void main(String[] args) {
    System.out.println(DSL.using(SQLDialect.MYSQL)
        .createTable("t")
        .column("b", BINARY)
        .column("v", VARBINARY));
}

Yielding:

create table `t`(
  `b` text null,
  `v` text null
)

However, this:

public static void main(String[] args) {
    System.out.println(DSL.using(SQLDialect.MYSQL)
        .createTable("t")
        .column("b", BINARY(256))
        .column("v", VARBINARY(256)));
}

Produces the wanted result:

create table `t`(
  `b` binary(256) null,
  `v` varbinary(256) null
)

So, a workaround would be to put a length in your VARBINARY column specification:

CREATE TABLE `TestTable`
(
  `Foo` BINARY(256),
  `Bar` VARBINARY(256)
)

@lukaseder
Copy link
Member

When fixing this similar to #6745, we should generate the BLOB type for binary data types.

@lukaseder
Copy link
Member

Fixed in jOOQ 3.13.0. Backport scheduled for 3.12.3 (#9477) (due today).

@lukaseder
Copy link
Member

This issue once more shows that we need to increase the priority of better data type management. Historically, we got away with what we have, because at runtime, more precise data type mappings are hardly needed. Even CAST expressions can get away with the occasional flaw.

But in DDL statements, we must be able to express data types, including vendor specific types, in a much more precise manner. We currently can't do that, e.g. we currently cannot do better than generating BLOB, when in fact you preferred VARBINARY (without length). I'm increasing the priority of #5713, to fix this more thoroughly.

@trdesilva
Copy link
Author

Thanks for the quick turnaround. It may be a separate issue, but there is one other thing I'd like to note: the BINARY column in my MCVE table has a length specified, but the generated Testtable class doesn't have a length on that column. Here's the SQL to create the table:

CREATE TABLE `TestTable`
(
  `Foo` BINARY(256),
  `Bar` VARBINARY
)

And here's the generated declaration for the Foo column in Testtable:

public final TableField<TesttableRecord, byte[]> FOO = createField(DSL.name("Foo"), org.jooq.impl.SQLDataType.VARBINARY, this, "");

I think this looks like a problem with the DDLDatabase code generator, but from your previous comment, I think it may also have contributed to my original problem.

@lukaseder
Copy link
Member

Thanks, @trdesilva: Indeed, it's always a good idea to create new issues if they're not strictly related, e.g. in order to track them. Your comment almost got lost in my inbox without ever being in any issue report.

If the issue persists, would you mind creating a new issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment