Skip to content

Conversation

@AdalbertMemSQL
Copy link
Collaborator

This PR adds the same functionality as a corresponding PR from JDBC
memsql/S2-JDBC-Connector#22
For simplicity, the vectorTypeOutputFormat configuration is not added (the same functionality can be achieved using session variables).

.findFirst()
.orElseThrow(() -> new IllegalArgumentException("Wrong extended vector type: " + code));
}
} No newline at end of file

Choose a reason for hiding this comment

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

nit: new line

VARSTRING(253),
STRING(254),
GEOMETRY(255);
GEOMETRY(255),

Choose a reason for hiding this comment

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

unrelated to PR: do you plan to remove GEOMETRY, or to modify it to correspond to our Geography(point), or just leave it as is?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

At the moment, our Geography(point) data type corresponds to the STRING type in the MySQL client/server protocol. I think it is a nice mapping and would like to leave it as it is.
The GEOMETRY type can be safely removed as it is never actually used.
I believe that the same can be done with OLDDECIMAL, NEWDATE, and NULL

public DataType getBinaryEncodeType() {
return DataType.BLOB;
}
} No newline at end of file

Choose a reason for hiding this comment

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

nit: new line

throw new IllegalArgumentException("Unsupported vector type: " + dataType.name());
}
}
} No newline at end of file

Choose a reason for hiding this comment

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

nit: new line

@AdalbertMemSQL AdalbertMemSQL merged commit e010307 into master Nov 19, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants