Document allow-drop-table property in JDBC connectors

[jhlodin]

As of Trino 368 at least, the allow-drop-table property must be enabled to allow DROP TABLE statements. This is at least true for JDBC connectors, need clarification if this applies to others.

[hashhar]

Correct. Only applies to JDBC connectors.

Note that we will revisit this once/if #588 makes in.

I have yet to check the list of JDBC connectors that support this.

[hashhar]

Add phoenix - but it has the default value set to true.

Other than that only druid is missing in this PR.

[hashhar]

Note that other connectors (like Hive and Cassandra) have "equivalent" configs (e.g. hive.allow-drop-table and cassandra.allow-drop-table) but they just happen to have the similar name as this config and are unrelated otherwise.

[jhlodin]

@hashhar Added a custom version of the fragment to Phoenix, noting that it's enabled by default but can be disabled.

I skipped over adding the fragment to Druid because it's read-only, so there's no point in enabling the property.

[electrum]

We're going to drop this property. I need to update and merge #588.

[jhlodin]

We're going to drop this property. I need to update and merge #588.

Thanks @electrum . Do you have a target release for that PR? If that's still TBD, I think for the sake of user experience we ought to document this behavior and revert once #588 is live. Reason being, we say that DROP TABLE is supported in these connectors, but when the user attempts to run that statement they get a generic "DROP TABLE is not allowed" error with no guidance on how to allow it.

[mosabua]

I am in favour of dropping the property, but given that we keep getting questions on slack and other forums we should get either the docs updated or the code changed merged asap.

[jhlodin]

Chatted with @electrum , the goal is to get #588 merged for 370. If it doesn't make that release, we agreed to merge this docs PR. So this can sit tight until it's either merged or closed with 370.

[jhlodin]

Looks like #588 is merged! @electrum Should I take that as confirmation that it's ok to close this PR?

[jhlodin]

Closing as the PR to remove this property has been merged.

[electrum]

Thanks @jhlodin for reminding me to get this change in. This legacy and confusing property is finally gone!