add xmltype in postgres

[mauravan]

Motivation:

see #1111

I'm looking for a little guidance on this.

1. where can i find an integration test i could use to test against while implementing?
2. maybe some pointers on where to start

thanks
R

[vietj]

@mauravan you can look at the existing tests of other PG specific data type and add tests in their location

[vietj]

I'm curious, did you try using a string ?

[mauravan]

that's why im looking for an integration test - i suspect we might be able to use a String as long as we pass it as UTF8. Then we could just remove the wrapper class.

i did not have a lot of time to look into it, but i will have some time soon and will dig a bit deeper

[mauravan]

Hi @vietj,

So turns out we might as well use String. Or do you think there is value in an extra class like the PgSQLXML i added? otherwise i will happily remove it.

Also what needs to happen next to get this merged?

Thanks for your help,
R

EDIT: I just tried this with quarkus and i think it's really unhandy to have to use an extra class for this when we could be using String - will change this

EDIT2: So i did it using String now and it works just fine i think - one problem i encountered was the mapping in the DataType.java because there is already a mapping for String to Varchar

encodingTypeToDataType.put(String.class, VARCHAR); encodingTypeToDataType.put(String[].class, VARCHAR_ARRAY); encodingTypeToDataType.put(String.class, XML); encodingTypeToDataType.put(String[].class, XML_ARRAY);

Which seems to be only important in case of a PrepareStatementCommand with parameterTypes which the test i'm executing are apparently not using. Maybe you have some more insight into that?

[vietj]

this needs to be reviewed to be merged

[vietj]

why is this commented ?

[mauravan]

See the Comment from the 22. of Dec:

So i did it using String now and it works just fine i think - one problem i encountered was the mapping in the DataType.java because there is already a mapping for String to Varchar

encodingTypeToDataType.put(String.class, VARCHAR); encodingTypeToDataType.put(String[].class, VARCHAR_ARRAY); encodingTypeToDataType.put(String.class, XML); encodingTypeToDataType.put(String[].class, XML_ARRAY);

Which seems to be only important in case of a PrepareStatementCommand with parameterTypes which the test i'm executing are apparently not using. Maybe you have some more insight into that?

[vietj]

this PR changes a lot of things that should not be changed (like reformatting), can you keep only to the essential changes ?

[mauravan]

this PR changes a lot of things that should not be changed (like reformatting), can you keep only to the essential changes ?

Hi @vietj, thanks for the feedback - i reverted all the changes.

The one big Problem with the private static final IntObjectMap<DataType> oidToDataType = new IntObjectHashMap<>(); remains. this structure does not allow for a duplicate mapping of String.

One solution i could think of is to use a wrapper class for XML which would then just hold the String. Do you have any other ideas?

[vietj]

why is it an issue ?

[mauravan]

There is these 2 cases for the Map:

static DataType valueOf(int oid) {
    DataType value = oidToDataType.get(oid);
    if (value == null) {
      logger.debug("Postgres type OID=" + oid + " not handled - using unknown type instead");
      return UNKNOWN;
    } else {
      return value;
    }
  }

  static DataType lookup(Class<?> type) {
    DataType dataType = encodingTypeToDataType.get(type);
    if (dataType == null) {
      if (Buffer.class.isAssignableFrom(type)) {
        return BYTEA;
      }
      dataType = DataType.UNKNOWN;
    }
    return dataType;
  }

Where for the valueOf Method we have a mapping for each oid for the lookup i think there can only be a mapping per type. So either the Mapping is

encodingTypeToDataType.put(String.class, XML); encodingTypeToDataType.put(String[].class, XML_ARRAY);

Or it is
encodingTypeToDataType.put(String.class, VARCHAR); encodingTypeToDataType.put(String[].class, VARCHAR_ARRAY);
The lookup seems to be called during encoding and the valueOf during decoding. So it seems to me, that encoding of String -> varchar would be disabled?

[vietj]

why is this commented ?

[mauravan]

As far as I understand it, there can only be one mapping per class to a Type. This is the Problem i was talking about in the other comments which we need to solve here - so either there will be a need for a wrapper class or someone can come up with a better solution for this

[vietj]

you are totally right, this is because we cannot specify which database type to use in a tuple for a given java type

[mauravan]

So there will be something like a new Class like PgXMLType which just holds a String. Would you agree to that?