Added support for MariaDb extend type info #288
Conversation
| @@ -96,6 +100,10 @@ public int getDefinitions() { | |||
| public short getDecimals() { | |||
| return decimals; | |||
| } | |||
|
|
|||
| collationId + ", size=" + size + ", type=" + typeId + ", definitions=" + definitions + | ||
| ", decimals=" + decimals + '}'; | ||
| originTable + "'), column='" + column + "' (origin:'" + originColumn + "'), extendedTypeInfo=" + | ||
| extendedTypeInfo + ", collationId=" +collationId + ", size=" + size + ", type=" + typeId + |
There was a problem hiding this comment.
Please wrap the string in single quotes '.
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| int result = 31 * typeId + (int) definitions; | ||
| return 31 * result + collationId; | ||
| return 31 * result + collationId + extendedTypeInfo.hashCode(); |
There was a problem hiding this comment.
Looks like it's not null-safe. Could you make it null-safe and apply *31 as well?
ex)
result = 31 * result + collationId;
return 31 * result + (extendedTypeInfo == null ? 0 : extendedTypeInfo.hashcode)
| } | ||
|
|
||
| @Override | ||
| public String toString() { | ||
| return "MySqlTypeMetadata{typeId=" + typeId + | ||
| ", definitions=0x" + Integer.toHexString(definitions) + | ||
| ", collationId=" + collationId + | ||
| ", collationId=" + collationId + | ||
| ", extendedTypeInfo=" + extendedTypeInfo + |
There was a problem hiding this comment.
single quote is missing (')
|
|
||
| @Override | ||
| public boolean isMariaDbJson() { | ||
| return extendedTypeInfo.equals("json"); |
| assertThat(metadata.isBinary()).isFalse(); | ||
| } | ||
|
|
||
| @Test |
There was a problem hiding this comment.
Could you add an integration test as well to make sure everything works?
There was a problem hiding this comment.
Hi @jchrys ,
I've added an integration test in MariaDbIntegrationTestSupport but I'm unsure of how to set/enable the extended type info capability for the test server. Do you know how to set this so that the test won't fail?
There was a problem hiding this comment.
Hello, @svats0001.
Extended type info was introduced in mariadb 10.5. I believe adding @EnabledIf("envIsMariaDb10_5_1") would accommodate this.
|
Hi @jchrys , I appreciate the feedback and I'll incorporate those changes in a new commit as well as the checkstyle changes. I'm having some temporary issues running checkstyle in my Eclipse IDE so as soon as I figure that out I'll resubmit the changes in some time. |
|
Hi, @svats0001. |
Hi @mirromutth ,
#254
Motivation:
If Capability.MARIADB_CLIENT_EXTENDED_TYPE_INFO is set, the Column Defintion Packet will contain extended type information. MariaDb returns JSON data as VARCHAR (MYSQL_TYPE_STRING) so reading the extended type information can be useful to identify the correct MySqlType.
Modification:
Capability: Uncommented MARIADB_CLIENT_EXTENDED_TYPE_INFO and added method that checks if this capability is enabled in bitmap.
MySqlColumnDescriptor: Altered constructor parameter to include nullable extendedTypeInfo field that is used to create MySqlTypeMetadata.
MySqlTypeMetadata: Added nullable extendedTypeInfo field and added isMariaDbJson method to check if the value of extendedTypeInfo equals 'json'. Included extendedTypeInfo in equals, hashcode and toString methods.
MySqlNativeTypeMetadata: Added isMariaDbJson method to check if the value of extendedTypeInfo equals 'json'.
MySqlType: In the MySqlType.of method, if type_id is ID_VARCHAR, ID_VAR_STRING or ID_STRING, returns VARBINARY if data is binary and if not returns JSON if isMariaDbJson() is true, otherwise returns VARCHAR.
DefinitionMetadataMessage: Added nullable extendedTypeInfo field. Added getter for this field and included it in hashcode, equals and toString methods. In decode41 method, if isMariaDb and isExtendedTypeInfo capabilities are enabled, added extendTypeInfo field that reads extended type information from buf.
MySqlRowDescriptorTest: Added null in MySqlColumnDescriptor argument to accommodate changes made to MySqlColumnDescriptor.
MySqlTypeMetadataTest: Added test method that checks if metadata isMariaDbJson is true/false depending on the value of extendedTypeInfo. Also checks if MYSQL_TYPE_STRING returns correct MySqlType (JSON/VARCHAR) depending on if extendedTypeInfo is 'json' or not.
Result:
If extended type information capability is enabled for MariaDb and a MariaDb JSON column is returned, the column should have a MySqlType of JSON rather than VARCHAR.
The drawbacks are that the extendedTypeInfo field has been added to several classes even though it's often null, which is always the case when using MySql and often the case when using MariaDb (only not null when extended type info capability is enabled and using protocol 4.1). Also even when it's not null, it only affects MySqlType when the value is 'json'.
Added a new test for MySqlTypeMetadata.