Add pinot query options to query

[naman-patel]

Description

Currently no Pinot query options can be passed along with query. This means Pinot null handling cant be leveraged. This PR provides a way to provide these query options.

Additional context and related issues

The same PR was also implemented for Presto. Here is the reference to that.

Fixes #21897

Release notes

(x) Release notes are required, with the following suggested text:

# Pinot
* Allow users to configure Pinot query options. ({21897}`issuenumber`)

.

[ebyhr]

Please add a new assertion to BasePinotConnectorSmokeTest#testQueryOptions with query_options session property.

[ebyhr]

We can define the variable like below and use it in assertion method.

        Session queryOptions = Session.builder(getQueryRunner().getDefaultSession())
                .setCatalogSessionProperty("pinot", "query_options", "...")
                .build();

Please refer to BasePinotConnectorSmokeTest.testAggregationPushdown.

[naman-patel]

This is connector config. I tried session catalog property. It complains its not a recognized session property.

[ebyhr]

I can use the above code. How did you try?

[naman-patel]

Exacty what you have there. It compiles but since that is not catalog session property, I see a runtime exception.

Session queryOptions = Session.builder(getQueryRunner().getDefaultSession())
                .setCatalogSessionProperty("pinot", "query_options", "skipUpsert=true")
                .build();
        assertThat(query(queryOptions,"SELECT city, \"sum(long_number)\" FROM" +
                         " \"SET skipUpsert = 'true';" +
                         " SET numReplicaGroupsToQuery = '1';" +
                         " SELECT city, SUM(long_number)" +
                         "  FROM my_table" +
                         "  GROUP BY city" +
                         "  HAVING SUM(long_number) > 10000\""))
                .matches("VALUES (VARCHAR 'Los Angeles', DOUBLE '50000.0'), (VARCHAR 'New York', DOUBLE '20000.0')")
                .isFullyPushedDown();

[ebyhr]

The session value skipUpsert=true should be skipUpsert:true.

[naman-patel]

@ebyhr addrressed your feedback except for the last one as I dont see a way to add those session properties in test framework.

[findinpath]

Use

trino/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/thrift/ThriftHttpMetastoreConfig.java

Lines 83 to 113 in df454db

	public Map<String, String> getAdditionalHeaders()
	{
	return additionalHeaders;
	}
	@Config("hive.metastore.http.client.additional-headers")
	@ConfigDescription("Comma separated key:value pairs to be send to metastore as additional headers")
	public ThriftHttpMetastoreConfig setAdditionalHeaders(String httpHeaders)
	{
	try {
	// we allow escaping the delimiters like , and : using back-slash.
	// To support that we create a negative lookbehind of , and : which
	// are not preceded by a back-slash.
	String headersDelim = "(?<!\\\\),";
	String kvDelim = "(?<!\\\\):";
	Map<String, String> temp = new HashMap<>();
	if (httpHeaders != null) {
	for (String kv : httpHeaders.split(headersDelim)) {
	String key = kv.split(kvDelim, 2)[0].trim();
	String val = kv.split(kvDelim, 2)[1].trim();
	temp.put(key, val);
	}
	this.additionalHeaders = ImmutableMap.copyOf(temp);
	}
	}
	catch (IndexOutOfBoundsException e) {
	throw new IllegalArgumentException(String.format("Invalid format for 'hive.metastore.http.client.additional-headers'. " +
	"Value provided is %s", httpHeaders), e);
	}
	return this;
	}

as PoC on how to deal with a config value containing a map of values.

[ebyhr]

Could you rebase on master to resolve conflicts?