Map Cassandra INET to Trino IPADDRESS type

[chen-ni]

Description

Is this change a fix, improvement, new feature, refactoring, or other?

An improvement in type mapping. We used to map Cassandra INET type to Trino VARCHAR. It would be better to map it to Trino IPADDRESS.

Due to the fact that IpAddressType is not located in io.trino.spi.type, we have to use typeManager().getType() to get the type reference (please refer to this discussion).

Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific)

The Cassandra connector.

How would you describe this change to a non-technical end user or system administrator?

By mapping the Cassandra INET to Trino IPADDRESS type, when reading INET data in Cassandra, we can apply IP Address Functions without the need for casting.

Related issues, pull requests, and links

Fix #851.

I've referred to the 2 following PRs:

• Map Cassandra UUID to Trino UUID type #5231
• Add to Elastic connector IpAddress field #3347

Documentation

( ) No documentation is needed.
(x) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

( ) No release notes entries required.
( ) Release notes entries required with the following suggested text:

# Cassandra
* Map Cassandra `inet` type to Trino `ipaddress` type. ({issue}`851`)

[findinpath]

Pls remove the first two commits (they are useless if the second one reverts the first one).

[findinpath]

Please do take some time to fill in the response about the question:

How would you describe this change to a non-technical end user or system administrator?

[ebyhr]

I’d like to know if this approach is promising.

You can refactor CassandraTypes first to graduate static type mapping. We wanted to make the mapping dynamic for LIST, MAP and SET type either.

[ebyhr]

Please update "Data types" in cassandra.rst. Thanks for your contribution.

[chen-ni]

You can refactor CassandraTypes first to graduate static type mapping.

@ebyhr Thanks for the helpful suggestion!

To move away from the static type mapping pattern, the simplest approach I can think of is just to make those fields non-static, initialize them from the constructor (and maybe add some builders later to allow dynamically getting type mappings) and access those type mappings through an instance of CassandraTypes.

Is this what's expected? Or is some bigger change expected?

[ebyhr]

Is this what's expected? Or is some bigger change expected?

I expect larger change. We shouldn't initialize a CassandraTypes variable from CassandraMetadata.

[chen-ni]

We shouldn't initialize a CassandraTypes variable from CassandraMetadata.

@ebyhr I agree. I'm trying to figure out how to avoid it, but it seems a little difficult.

The biggest problem is how to get a typeManager instance in the static context of CassandraTypes. Perhaps we could consider using static injection in Guice? But it's not a recommended approach either.

As for the refactoring you suggested earlier:

You can refactor CassandraTypes first to graduate static type mapping.

I'm still not sure which kind of refactoring is expected. I'll keep trying, but it would be great if you could shed some light on it when possible :)

Anyway, now that the tests pass, I'll keep refactoring until it's acceptable.

[chen-ni]

@ebyhr I just removed the static INET type in CassandraTypes altogether. Please check if this approach is acceptable. Thanks!

[findinpath]

nit: separate commit CassandraType.toCassandraType -> toCassandraType

[chen-ni]

@findinpath Sorry, I don't quite understand here. What do we need to put in the separate commit? A change to static import?

[ebyhr]

Merged, thanks!

[chen-ni]

@ebyhr Sorry, just realized I didn't remove the outdated static INET type here:

trino/plugin/trino-cassandra/src/main/java/io/trino/plugin/cassandra/CassandraTypes.java

Line 51 in f8be32e

public static final CassandraType INET = new CassandraType(Kind.INET, createVarcharType(IP_ADDRESS_STRING_MAX_LENGTH));

It was indeed removed in the original commit but the change must have been lost while resolving a conflict during one of the fixes later.

Should I create a simple PR to remove it or it's fine to leave it like this for a while?

P.S. Thanks for the helpful code reviews and refactoring example :)

[ebyhr]

@chen-ni You can send another PR. Thanks for finding that.