Document "DROP TABLE IF EXISTS" throws an error if view exists

[sushi30]

folliowing #11555

Description

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

documentation

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

no

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

add documentation regarding an error

Related issues, pull requests, and links

• Follow Prevent DROP/RENAME TABLE IF EXISTS from silently ignoring views #11555

Documentation

(x) No documentation is needed.
( ) 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

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

# Section
* Fix some things. ({issue}`issuenumber`)

[cla-bot]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to [email protected]. For more information, see https://github.com/trinodb/cla.

[colebow]

LGTM. If you haven't already submitted the CLA, ping me here and I'll try to communicate that to the maintainers once you have.

[mosabua]

The error is not supressed

[mosabua]

But I am also wondering if that is true ... what kind of view are we talking about here? A trino native view or a view in the underlying database? And is this consistent across connectors..

[mosabua]

@hashhar can you clarify how this works for views (and what kind of views)

[hashhar]

Trino doesn't know if something is a view in remote database (at-least for JDBC connectors or things like BigQuery). So those views are tables for Trino. i.e. DROP TABLE will work on views that exist in remote system.

This is specifically talking about Trino views.

cc: @findinpath for double checking me regarding other connectors like hive.

[mosabua]

So we most likely have to clarify what type of view we are talking about here .. since from @hashhar is saying .. this current change would be incorrect for views in JDBC connector data sources and similar.

Also thinking about this some more I am wondering why we want to add this message at all. E.g. will it actually drop a native view if is exists with the "Drop table" command? This might require a larger discussion on how this works and how we document that consistent across the system.

[sushi30]

So we most likely have to clarify what type of view we are talking about here .. since from @hashhar is saying .. this current change would be incorrect for views in JDBC connector data sources and similar.

Also thinking about this some more I am wondering why we want to add this message at all. E.g. will it actually drop a native view if is exists with the "Drop table" command? This might require a larger discussion on how this works and how we document that consistent across the system.

The documentation currently makes a blanket statement that errors are suppressed which is clearly wrong. What is more "wrong"? Current documentation or the proposed documentation? I think that this is a step in right direction but if it's a larger issue then I suggest someone with a deeper understanding open an issue and lead a discussion. I use trino only with hive so I do not have the context mentioned in this discussion like JDBC.

[mosabua]

Oh.. so this applies to Hive views or Trino views in your case? Or both ...

The thing is that for JDBC connectors where a table and a view (native view in the datasource) are the same .. and the documentation for that use case is correct.. by adding your PR we would make it wrong in that case.. I hope that explains what the problem is and why we need to decide how to properly fix this.

[sushi30]

Trino views with hive connector (not sure how they are represented by Hive under the hood).

[findinpath]

Relevant code for DROP TABLE functionality.

trino/core/trino-main/src/main/java/io/trino/execution/DropTableTask.java

Lines 65 to 84 in 57983d9

	if (metadata.isMaterializedView(session, originalTableName)) {
	throw semanticException(
	GENERIC_USER_ERROR,
	statement,
	"Table '%s' does not exist, but a materialized view with that name exists. Did you mean DROP MATERIALIZED VIEW %s?", originalTableName, originalTableName);
	}
	if (metadata.isView(session, originalTableName)) {
	throw semanticException(
	GENERIC_USER_ERROR,
	statement,
	"Table '%s' does not exist, but a view with that name exists. Did you mean DROP VIEW %s?", originalTableName, originalTableName);
	}
	RedirectionAwareTableHandle redirectionAwareTableHandle = metadata.getRedirectionAwareTableHandle(session, originalTableName);
	if (redirectionAwareTableHandle.getTableHandle().isEmpty()) {
	if (!statement.isExists()) {
	throw semanticException(TABLE_NOT_FOUND, statement, "Table '%s' does not exist", originalTableName);
	}
	return immediateVoidFuture();

Note that if we're trying to drop via DROP TABLE a materialized view or a view , a generic user error will be thrown.

If the table to be dropped does not exist, the statement has no effect.

I find the following statement misleading because there is no mention to the table not found error to be suppressed:

The optional IF EXISTS clause causes the error to be suppressed if the table does not exist.

[mosabua]

So @findinpath and @hashhar .. how do we go from here.. I am not sure this doc change on its own is sufficient or even correct.

[sushi30]

LGTM. If you haven't already submitted the CLA, ping me here and I'll try to communicate that to the maintainers once you have.

I have signed the CLA and sent it over.

[cla-bot]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to [email protected]. For more information, see https://github.com/trinodb/cla.

[mosabua]

Thank you for the CLA.. we will process shortly while we continue to sort out the content of the PR.

[mosabua]

This is good to go. I will follow up with more clarifying docs about views and Trino views and so on. I also got a training session for this ready to go and will work the material into the docs.