Document existing Hive properties by findinpath · Pull Request #10831 · trinodb/trino

findinpath · 2022-01-27T21:09:42Z

No description provided.

docs/src/main/sphinx/connector/hive.rst

findinpath · 2022-01-28T07:00:27Z

docs/src/main/sphinx/connector/hive.rst

Fixes #6437

Currently it just says "if you want faster, enable this", which isn't the whole truth
The documentation should say when this is ok to set and when it's not.

@findepi can you help me come up with a better description here?

Maybe

Allow the metastore to assume that partition keys are normalized for non-string partition keys. This can lead to performance improvements for affected tables. Not that ..

Changed to

Allow the metastore to assume that the values of partition
columns can be converted to string values. This can lead to
performance improvements in the queries which apply filters
on the partition columns.
Note that the partition keys of type timestamp do
not get canonicalized. Default is false.

findepi · 2022-01-28T09:20:04Z

docs/src/main/sphinx/connector/hive.rst

Currently it just says "if you want faster, enable this", which isn't the whole truth
The documentation should say when this is ok to set and when it's not.

findepi · 2022-01-28T09:20:39Z

docs/src/main/sphinx/connector/hive.rst

The documentation should say when this is ok to set and when it's not.

I am missing knowledge in this area. For which types would it be a bad idea to set the parameter to true ?
Is there any automated test showcasing the limitations?

mosabua

This is a great addition.

docs/src/main/sphinx/connector/hive.rst

mosabua · 2022-04-07T18:13:35Z

docs/src/main/sphinx/connector/hive.rst

Here and in other places below .. only one space character before code or between sentences

In this specific case true is part of of the "Default" column and not the description sentence.

docs/src/main/sphinx/connector/hive.rst

mosabua · 2022-04-07T18:17:07Z

docs/src/main/sphinx/connector/hive.rst

Is that true .. does the HMS actually run the delete of the actual data? I thought it just manages the metadata...

In case of dealing with tables of type MANAGED_TABLE , when dropping the table, HMS is responsible of dropping the content of the table as well. Do note that AWS Glue metastore does not provide the same functionality when dropping tables.

mosabua · 2022-04-07T18:22:00Z

docs/src/main/sphinx/connector/hive.rst

Maybe

Allow the metastore to assume that partition keys are normalized for non-string partition keys. This can lead to performance improvements for affected tables. Not that ..

mosabua · 2022-04-07T18:22:45Z

docs/src/main/sphinx/connector/hive.rst

URL of the SOCKS proxy to use to for connecting to the Thrift Hive metastore.

mosabua · 2022-04-07T18:23:29Z

docs/src/main/sphinx/connector/hive.rst

running. Defaults to

same for the various other ones like that below

mosabua · 2022-04-07T18:24:35Z

docs/src/main/sphinx/connector/hive.rst

Related question not to be answered here...

how to you add access to that class.. drop the related jars into the Hive connector plugin folder?

If this is a common use case we might need to document specifically .. or generically somewhere

trino/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/GlueCredentialsProvider.java

Lines 66 to 78 in f7e0a0d

private static AWSCredentialsProvider getCustomAWSCredentialsProvider(String providerClass)

{

try {

Object instance = Class.forName(providerClass).getConstructor().newInstance();

if (!(instance instanceof AWSCredentialsProvider)) {

throw new RuntimeException("Invalid credentials provider class: " + instance.getClass().getName());

}

return (AWSCredentialsProvider) instance;

}

catch (ReflectiveOperationException e) {

throw new RuntimeException(format("Error creating an instance of %s", providerClass), e);

}

}

I'm assuming that this setting was thought to deal with one of the many credential providers which come with the aws-java-sdk-core library.

If the class is not in the library aws-java-sdk-core then it would need to be probably added to hive's plugin folder.

Related PR: #1363

docs/src/main/sphinx/connector/hive.rst

mosabua · 2022-10-25T15:27:53Z

@findinpath will you pick this up again and progress it towards merge ..

Fyi @m57lyra is leading a plan to heavily refactor the hive connector docs.

findinpath · 2022-10-25T15:39:11Z

@mosabua I think that this PR can be closed then. Sorry for keeping it open for so long. I lost in the meantime the context to the PR and would very likely need to start over from scratch.

mosabua · 2022-10-25T15:41:26Z

@colebow can you harvest whatever is useful from this PR and still missing in our docs and do a lift and shift... it would be a shame to loose all the good work from @findinpath .. to keep it simple we could break it up into a whole bunch of small PR that can be merged quickly and that can be spread across the team

cla-bot bot added the cla-signed label Jan 27, 2022

findinpath requested review from mosabua and rosewms January 27, 2022 21:09

findinpath added the docs label Jan 27, 2022

findinpath requested review from findepi and posulliv January 27, 2022 21:10

findinpath force-pushed the docs/hive-properties branch from d659d33 to 5d5cbca Compare January 27, 2022 21:14

findinpath commented Jan 27, 2022

View reviewed changes

docs/src/main/sphinx/connector/hive.rst Outdated Show resolved Hide resolved

mosabua removed the request for review from rosewms January 27, 2022 22:49

hashhar reviewed Jan 28, 2022

View reviewed changes

docs/src/main/sphinx/connector/hive.rst Outdated Show resolved Hide resolved

findinpath force-pushed the docs/hive-properties branch from 5d5cbca to dc8cf74 Compare January 28, 2022 06:59

findinpath commented Jan 28, 2022

View reviewed changes

findepi reviewed Jan 28, 2022

View reviewed changes

findinpath force-pushed the docs/hive-properties branch from 2b6def3 to e8ff50f Compare January 30, 2022 13:22

findinpath requested a review from findepi January 31, 2022 06:48

findinpath mentioned this pull request Feb 3, 2022

Document hive.single-statement-writes #10899

Open

mosabua approved these changes Feb 7, 2022

View reviewed changes

posulliv reviewed Feb 8, 2022

View reviewed changes

docs/src/main/sphinx/connector/hive.rst Outdated Show resolved Hide resolved

findinpath force-pushed the docs/hive-properties branch from 8f75a47 to e5803f7 Compare February 9, 2022 05:36

findinpath force-pushed the docs/hive-properties branch from e5803f7 to 115d1c9 Compare February 23, 2022 12:50

findepi requested review from mosabua and posulliv March 25, 2022 12:18

mosabua requested changes Apr 7, 2022

View reviewed changes

findinpath added 7 commits September 8, 2022 07:03

Document Hive metastore thrift settings

4da36c2

Expand "Property Name" section for AWS Glue settings

d1bd1e9

Break default value to separate sentence

e29dbd1

Document hive.metastore.glue.assume-canonical-partition-keys setting

c89aebb

Align "Default" content for Hive configuration settings

213a27a

Document hive.validate-bucketing setting

42b1bc5

Document hive.single-statement-writes setting

888d9be

findinpath force-pushed the docs/hive-properties branch from 115d1c9 to 888d9be Compare September 8, 2022 05:28

findinpath requested a review from mosabua September 8, 2022 05:28

findinpath closed this Oct 25, 2022

colebow mentioned this pull request Jan 12, 2023

Hive config properties docs improvements #15700

Closed

colebow mentioned this pull request Mar 30, 2023

Add missing config properties to Hive docs #16808

Merged

	private static AWSCredentialsProvider getCustomAWSCredentialsProvider(String providerClass)
	{
	try {
	Object instance = Class.forName(providerClass).getConstructor().newInstance();
	if (!(instance instanceof AWSCredentialsProvider)) {
	throw new RuntimeException("Invalid credentials provider class: " + instance.getClass().getName());
	}
	return (AWSCredentialsProvider) instance;
	}
	catch (ReflectiveOperationException e) {
	throw new RuntimeException(format("Error creating an instance of %s", providerClass), e);
	}
	}

Conversation

findinpath commented Jan 27, 2022

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mosabua left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mosabua commented Oct 25, 2022

Uh oh!

findinpath commented Oct 25, 2022

Uh oh!

mosabua commented Oct 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

5 participants

mosabua commented Oct 25, 2022 •

edited

Loading