Y-Wakuta commented on code in PR #4281: URL: https://github.com/apache/polaris/pull/4281#discussion_r3198495948
########## persistence/relational-jdbc/src/main/resources/mysql/schema-v4.sql: ########## @@ -0,0 +1,245 @@ +-- +-- Licensed to the Apache Software Foundation (ASF) under one +-- or more contributor license agreements. See the NOTICE file-- +-- distributed with this work for additional information +-- regarding copyright ownership. The ASF licenses this file +-- to you under the Apache License, Version 2.0 (the +-- "License"). You may not use this file except in compliance +-- with the License. You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, +-- software distributed under the License is distributed on an +-- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +-- KIND, either express or implied. See the License for the +-- specific language governing permissions and limitations +-- under the License. + +-- MySQL schema v4 (matching PostgreSQL schema v4) +-- Design notes: +-- * The POLARIS_SCHEMA database must be pre-created and the user must have privileges on it. +-- * In MySQL, schema is synonymous with database. +-- * Indexes are declared inside CREATE TABLE because MySQL does not support +-- "CREATE INDEX IF NOT EXISTS" — the bootstrap runs this script once per realm +-- and "CREATE TABLE IF NOT EXISTS" handles the idempotency. +-- * Uses VARCHAR(255) for PRIMARY KEY / UNIQUE KEY columns where PostgreSQL uses TEXT +-- (MySQL forbids TEXT columns in keys without a prefix length). +-- * Uses JSON type instead of JSONB. +-- * Uses ON DUPLICATE KEY UPDATE instead of ON CONFLICT ... DO UPDATE. +-- * Requires MySQL server to be started with lower_case_table_names=1 +-- (Polaris code uses both UPPERCASE and lowercase table identifiers). + +USE POLARIS_SCHEMA; + +CREATE TABLE IF NOT EXISTS version ( + version_key VARCHAR(255) PRIMARY KEY COMMENT 'version key', + version_value INTEGER NOT NULL COMMENT 'version value' +) COMMENT = 'the version of the JDBC schema in use'; + +INSERT INTO version (version_key, version_value) +VALUES ('version', 4) +ON DUPLICATE KEY UPDATE version_value = VALUES(version_value); + +CREATE TABLE IF NOT EXISTS entities ( + realm_id VARCHAR(255) NOT NULL COMMENT 'realm_id used for multi-tenancy', + catalog_id BIGINT NOT NULL COMMENT 'catalog id', + id BIGINT NOT NULL COMMENT 'entity id', + parent_id BIGINT NOT NULL COMMENT 'entity id of parent', + -- Matches Polaris's MAX_IDENTIFIER_LENGTH = 256. Keeps the UNIQUE + -- (realm_id, ..., name) index well within the InnoDB 3072-byte key-length + -- limit: 255*4 + 8+8+4 + 256*4 = 2064 bytes (utf8mb4). + name VARCHAR(256) NOT NULL COMMENT 'entity name', + entity_version INT NOT NULL COMMENT 'version of the entity', + type_code INT NOT NULL COMMENT 'type code', + sub_type_code INT NOT NULL COMMENT 'sub type of entity', + create_timestamp BIGINT NOT NULL COMMENT 'creation time of entity', + drop_timestamp BIGINT NOT NULL COMMENT 'time of drop of entity', + purge_timestamp BIGINT NOT NULL COMMENT 'time to start purging entity', + to_purge_timestamp BIGINT NOT NULL, + last_update_timestamp BIGINT NOT NULL COMMENT 'last time the entity is touched', + properties JSON NOT NULL DEFAULT ('{}') COMMENT 'entities properties json', + internal_properties JSON NOT NULL DEFAULT ('{}') COMMENT 'entities internal properties json', + grant_records_version INT NOT NULL COMMENT 'the version of grant records change on the entity', + location_without_scheme TEXT, + PRIMARY KEY (realm_id, id), + CONSTRAINT constraint_name UNIQUE (realm_id, catalog_id, parent_id, type_code, name), Review Comment: @adutra Agreed that `constraint_name` is an unnatural name. However, this same name is currently used in the postgres, h2, and cockroachdb schemas as well. One option would be to rename it to something like `uq_entities_identity`, but that would introduce a divergence between the MySQL schema and the others that isn't driven by any DB-specific behavior. Long-term I think renaming the constraint across all DB schemas is the right move, but I'd like to avoid changing the non-MySQL schemas in this PR. Which of the following do you think is more appropriate? 1. Use a better name (e.g. `uq_entities_identity`) only in the MySQL schema — improves the name, but introduces a temporary divergence from the other DBs. 2. Keep the same `constraint_name` as the other DBs in this PR — consistent across all schemas, defers the rename entirely. Either way, if we don't touch the other schemas here, I'd like to open a follow-up PR to rename the constraint across all DB schemas. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
