Copilot commented on code in PR #6965:
URL: https://github.com/apache/ignite-3/pull/6965#discussion_r2523453377
##########
modules/jdbc/src/main/java/org/apache/ignite/internal/jdbc/JdbcDatabaseMetadata.java:
##########
@@ -1354,23 +1362,23 @@ public ResultSet getIndexInfo(String catalog, String
schema, String tbl, boolean
boolean approximate) throws SQLException {
ensureNotClosed();
- final List<JdbcColumnMeta> meta = asList(
- new JdbcColumnMeta("TABLE_CAT", ColumnType.STRING),
- new JdbcColumnMeta("TABLE_SCHEM", ColumnType.STRING),
- new JdbcColumnMeta("TABLE_NAME", ColumnType.STRING),
- new JdbcColumnMeta("NON_UNIQUE", ColumnType.BOOLEAN),
- new JdbcColumnMeta("INDEX_QUALIFIER", ColumnType.STRING),
- new JdbcColumnMeta("INDEX_NAME", ColumnType.STRING),
- new JdbcColumnMeta("TYPE", ColumnType.INT16),
- new JdbcColumnMeta("ORDINAL_POSITION", ColumnType.INT16),
- new JdbcColumnMeta("COLUMN_NAME", ColumnType.STRING),
- new JdbcColumnMeta("ASC_OR_DESC", ColumnType.STRING),
- new JdbcColumnMeta("CARDINALITY", ColumnType.INT32),
- new JdbcColumnMeta("PAGES", ColumnType.INT32),
- new JdbcColumnMeta("FILTER_CONDITION", ColumnType.STRING));
+ List<ColumnMetadata> meta = asList(
+ columnMeta("TABLE_CAT", ColumnType.STRING),
+ columnMeta("TABLE_SCHEM", ColumnType.STRING),
+ columnMeta("TABLE_NAME", ColumnType.STRING),
+ columnMeta("NON_UNIQUE", ColumnType.BOOLEAN),
+ columnMeta("INDEX_QUALIFIER", ColumnType.STRING),
+ columnMeta("INDEX_NAME", ColumnType.STRING),
+ columnMeta("TYPE", ColumnType.INT16),
+ columnMeta("ORDINAL_POSITION", ColumnType.INT16),
+ columnMeta("COLUMN_NAME", ColumnType.STRING),
+ columnMeta("ASC_OR_DESC", ColumnType.STRING),
+ columnMeta("CARDINALITY", ColumnType.INT32),
+ columnMeta("PAGES", ColumnType.INT32),
+ columnMeta("FILTER_CONDITION", ColumnType.STRING));
if (!isValidCatalog(catalog)) {
- return new JdbcResultSet(Collections.emptyList(), meta);
+ return createObjectListResultSet(Collections.emptyList(), meta);
Review Comment:
[nitpick] The `createObjectListResultSet` method is called with
`Collections.emptyList()` when the metadata is already known to be empty. This
is inconsistent with other usages where an empty metadata list is created using
the single-argument overload (e.g., lines 906, 950, 1035, 1144). For
consistency, consider using `createObjectListResultSet(meta)` instead of
`createObjectListResultSet(Collections.emptyList(), meta)`.
```suggestion
return createObjectListResultSet(meta);
```
##########
modules/jdbc/src/main/java/org/apache/ignite/internal/jdbc/JdbcDatabaseMetataUtils.java:
##########
@@ -0,0 +1,343 @@
+/*
+ * 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.ignite.internal.jdbc;
+
+import java.math.BigDecimal;
+import java.sql.ResultSet;
+import java.time.Instant;
+import java.time.LocalDate;
+import java.time.LocalDateTime;
+import java.time.LocalTime;
+import java.time.ZoneId;
+import java.util.EnumSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.UUID;
+import org.apache.ignite.internal.jdbc2.ClientSyncResultSet;
+import org.apache.ignite.internal.jdbc2.JdbcResultSet;
+import org.apache.ignite.internal.sql.ResultSetMetadataImpl;
+import org.apache.ignite.internal.util.IgniteUtils;
+import org.apache.ignite.internal.util.TransformingIterator;
+import org.apache.ignite.sql.ColumnMetadata;
+import org.apache.ignite.sql.ColumnType;
+import org.apache.ignite.sql.NoRowSetExpectedException;
+import org.apache.ignite.sql.ResultSetMetadata;
+import org.apache.ignite.sql.SqlRow;
+import org.apache.ignite.table.Tuple;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Helper methods for creating a {@link ResultSet} using a list of objects.
+ */
+class JdbcDatabaseMetataUtils {
Review Comment:
The class name has a typo: "Metata" should be "Metadata". This affects the
class name `JdbcDatabaseMetataUtils` which should be
`JdbcDatabaseMetadataUtils`.
```suggestion
class JdbcDatabaseMetadataUtils {
```
##########
modules/jdbc/src/main/java/org/apache/ignite/internal/jdbc2/JdbcStatement2.java:
##########
@@ -510,12 +510,24 @@ public int getFetchDirection() throws SQLException {
return FETCH_FORWARD;
}
- /** {@inheritDoc} */
+ /**
+ * Gives the JDBC driver a hint as to the number of rows that should
+ * be fetched from the database.
+ *
+ * <p>Note: the current implementation does not provide any means to
change the number
+ * of rows after the statement has executed. Thus, changing this value
after the statement
+ * has executed will not apply to the previously retrieved ResultSet.
+ *
+ * @param fetchSize the number of rows to fetch
+ * @exception SQLException if the condition {@code rows >= 0} is not
satisfied.
Review Comment:
The javadoc has an issue: the `@exception` tag should use `fetchSize < 0`
instead of `rows >= 0` to match the actual validation in the code. The current
documentation states the condition as `rows >= 0` which is confusing since the
parameter is named `fetchSize`, and the validation checks if `fetchSize < 0`.
```suggestion
* @exception SQLException if {@code fetchSize < 0}.
```
##########
modules/jdbc/src/main/java/org/apache/ignite/internal/jdbc/JdbcDatabaseMetadata.java:
##########
@@ -24,6 +24,7 @@
import static java.sql.RowIdLifetime.ROWID_UNSUPPORTED;
import static java.util.Arrays.asList;
import static java.util.Collections.singletonList;
+import static
org.apache.ignite.internal.jdbc.JdbcDatabaseMetataUtils.createObjectListResultSet;
Review Comment:
The import statement contains a typo in the class name:
"JdbcDatabaseMetataUtils" should be "JdbcDatabaseMetadataUtils".
```suggestion
import static
org.apache.ignite.internal.jdbc.JdbcDatabaseMetadataUtils.createObjectListResultSet;
```
##########
modules/jdbc/src/main/java/org/apache/ignite/internal/jdbc/JdbcDatabaseMetataUtils.java:
##########
@@ -0,0 +1,343 @@
+/*
+ * 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.ignite.internal.jdbc;
+
+import java.math.BigDecimal;
+import java.sql.ResultSet;
+import java.time.Instant;
+import java.time.LocalDate;
+import java.time.LocalDateTime;
+import java.time.LocalTime;
+import java.time.ZoneId;
+import java.util.EnumSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.UUID;
+import org.apache.ignite.internal.jdbc2.ClientSyncResultSet;
+import org.apache.ignite.internal.jdbc2.JdbcResultSet;
+import org.apache.ignite.internal.sql.ResultSetMetadataImpl;
+import org.apache.ignite.internal.util.IgniteUtils;
+import org.apache.ignite.internal.util.TransformingIterator;
+import org.apache.ignite.sql.ColumnMetadata;
+import org.apache.ignite.sql.ColumnType;
+import org.apache.ignite.sql.NoRowSetExpectedException;
+import org.apache.ignite.sql.ResultSetMetadata;
+import org.apache.ignite.sql.SqlRow;
+import org.apache.ignite.table.Tuple;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Helper methods for creating a {@link ResultSet} using a list of objects.
+ */
+class JdbcDatabaseMetataUtils {
+ /** List of support metadata column types. */
Review Comment:
The comment on line 49 has a typo: "support" should be "supported".
```suggestion
/** List of supported metadata column types. */
```
##########
modules/jdbc/src/main/java/org/apache/ignite/internal/jdbc/JdbcDatabaseMetadata.java:
##########
@@ -813,22 +815,28 @@ public boolean dataDefinitionIgnoredInTransactions() {
return false;
}
+ private static ColumnMetadata columnMeta(String name, ColumnType type) {
+ return new ColumnMetadataImpl(name, type, -1, -1, true, null);
+ }
+
+
+
Review Comment:
[nitpick] There are extra blank lines added here (lines 822-823). While not
a functional issue, this creates unnecessary whitespace that could be removed
for consistency.
```suggestion
```
##########
modules/jdbc/src/main/java/org/apache/ignite/internal/jdbc/JdbcDatabaseMetadata.java:
##########
@@ -1777,14 +1785,14 @@ public static List<Object> columnRow(JdbcColumnMeta
colMeta, int pos) {
row.add(CATALOG_NAME); // 1. TABLE_CAT
row.add(colMeta.schemaName()); // 2. TABLE_SCHEM
row.add(colMeta.tableName()); // 3. TABLE_NAME
- row.add(colMeta.columnLabel()); // 4. COLUMN_NAME
- row.add(colMeta.dataType()); // 5. DATA_TYPE
+ row.add(colMeta.columnLabel()); // 4. COLUMN_NAME
+ row.add((short) colMeta.dataType()); // 5. DATA_TYPE
row.add(colMeta.dataTypeName()); // 6. TYPE_NAME
row.add(colMeta.precision() == -1 ? null : colMeta.precision()); // 7.
COLUMN_SIZE
row.add((Integer) null); // 8. BUFFER_LENGTH
row.add(colMeta.scale() == -1 ? null : colMeta.scale()); //
9. DECIMAL_DIGITS
row.add(10); // 10. NUM_PREC_RADIX
- row.add(colMeta.isNullable() ? columnNullable : columnNoNulls); //
11. NULLABLE
+ row.add(colMeta.isNullable() ? (short) columnNullable : (short)
columnNoNulls); // 11. NULLABLE
row.add((String) null); // 12. REMARKS
row.add(colMeta.defaultValue()); // 13. COLUMN_DEF
row.add(colMeta.dataType()); // 14. SQL_DATA_TYPE
Review Comment:
Inconsistent data type casting. Line 1789 casts `colMeta.dataType()` to
`short`, but line 1798 uses `colMeta.dataType()` without casting for the
`SQL_DATA_TYPE` field. Since these are both representing SQL data type
integers, they should use consistent types. According to JDBC specification,
both DATA_TYPE and SQL_DATA_TYPE should be INT32/int type.
--
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]
