Copilot commented on code in PR #9742: URL: https://github.com/apache/gravitino/pull/9742#discussion_r2703031379
########## catalogs/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/ClickHouseCatalogCapability.java: ########## @@ -0,0 +1,7 @@ +package org.apache.gravitino.catalog.clickhouse; Review Comment: Missing Apache License 2.0 header. All Java source files in Apache Gravitino must include the standard Apache License 2.0 header at the beginning of the file. ########## catalogs/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/converter/ClickHouseExceptionConverter.java: ########## @@ -0,0 +1,24 @@ +/* + * 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. + */ +package org.apache.gravitino.catalog.clickhouse.converter; + +import org.apache.gravitino.catalog.jdbc.converter.JdbcExceptionConverter; + +// TODO : Implement ClickHouse specific exception conversions if needed Review Comment: TODO comments should reference an issue number. According to the coding guidelines, TODO or FIXME comments must include a reference to a tracking issue (e.g., TODO(#ISSUE_NUMBER): description). ########## catalogs/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/converter/ClickHouseColumnDefaultValueConverter.java: ########## @@ -0,0 +1,24 @@ +/* + * 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. + */ +package org.apache.gravitino.catalog.clickhouse.converter; + +import org.apache.gravitino.catalog.jdbc.converter.JdbcColumnDefaultValueConverter; + +// TODO: Implement ClickHouse specific default value conversions if needed Review Comment: TODO comments should reference an issue number. According to the coding guidelines, TODO or FIXME comments must include a reference to a tracking issue (e.g., TODO(#ISSUE_NUMBER): description). ########## catalogs/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseDatabaseOperations.java: ########## @@ -0,0 +1,35 @@ +/* + * 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. + */ +package org.apache.gravitino.catalog.clickhouse.operations; + +import java.util.Set; +import org.apache.gravitino.catalog.jdbc.operation.JdbcDatabaseOperations; + +public class ClickHouseDatabaseOperations extends JdbcDatabaseOperations { + + @Override + protected boolean supportSchemaComment() { + return false; + } + + @Override + protected Set<String> createSysDatabaseNameSet() { + return null; Review Comment: Returning null will cause a NullPointerException when isSystemDatabase() is called. The parent class JdbcDatabaseOperations calls createSysDatabaseNameSet().contains() which will throw NPE if null is returned. Return an empty set (e.g., Collections.emptySet() or ImmutableSet.of()) if ClickHouse has no system databases to filter, or provide the appropriate set of system database names. ########## catalogs/catalog-jdbc-clickhouse/src/main/resources/META-INF/services/org.apache.gravitino.CatalogProvider: ########## @@ -0,0 +1,19 @@ +# +# 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 +# Review Comment: Inconsistent formatting in the Apache license header. Lines 9 and 11 have trailing spaces after the '#' character, while other JDBC catalog implementations (e.g., MySQL) have just '#' without trailing spaces on these lines. For consistency across the codebase, remove the trailing space. ```suggestion # # http://www.apache.org/licenses/LICENSE-2.0 # ``` -- 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]
