You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "skorotkov (via GitHub)" <gi...@apache.org> on 2024/08/22 10:10:58 UTC

[PR] IGNITE-6141 JDBC thin: support BLOB and CLOB types [ignite]

skorotkov opened a new pull request, #11492:
URL: https://github.com/apache/ignite/pull/11492

   Thank you for submitting the pull request to the Apache Ignite.
   
   In order to streamline the review of the contribution 
   we ask you to ensure the following steps have been taken:
   
   ### The Contribution Checklist
   - [ ] There is a single JIRA ticket related to the pull request. 
   - [ ] The web-link to the pull request is attached to the JIRA ticket.
   - [ ] The JIRA ticket has the _Patch Available_ state.
   - [ ] The pull request body describes changes that have been made. 
   The description explains _WHAT_ and _WHY_ was made instead of _HOW_.
   - [ ] The pull request title is treated as the final commit message. 
   The following pattern must be used: `IGNITE-XXXX Change summary` where `XXXX` - number of JIRA issue.
   - [ ] A reviewer has been mentioned through the JIRA comments 
   (see [the Maintainers list](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-ReviewProcessandMaintainers)) 
   - [ ] The pull request has been checked by the Teamcity Bot and 
   the `green visa` attached to the JIRA ticket (see [TC.Bot: Check PR](https://mtcga.gridgain.com/prs.html))
   
   ### Notes
   - [How to Contribute](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute)
   - [Coding abbreviation rules](https://cwiki.apache.org/confluence/display/IGNITE/Abbreviation+Rules)
   - [Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines)
   - [Apache Ignite Teamcity Bot](https://cwiki.apache.org/confluence/display/IGNITE/Apache+Ignite+Teamcity+Bot)
   
   If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com _#ignite_ channel.
   


-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-6141 JDBC: add basic support for BLOB and CLOB types [ignite]

Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #11492:
URL: https://github.com/apache/ignite/pull/11492#discussion_r1751789847


##########
modules/clients/src/test/java/org/apache/ignite/internal/jdbc2/JdbcResultSetSelfTest.java:
##########
@@ -670,6 +675,54 @@ public void testArray() throws Exception {
         assert cnt == 1;
     }
 
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testBlob() throws Exception {
+        ResultSet rs = stmt.executeQuery(SQL);
+
+        int cnt = 0;
+
+        while (rs.next()) {
+            if (cnt == 0) {
+                Blob blob = rs.getBlob("blobVal");
+                Assert.assertArrayEquals(blob.getBytes(1, (int)blob.length()), new byte[] {1});
+
+                blob = rs.getBlob(23);
+                Assert.assertArrayEquals(blob.getBytes(1, (int)blob.length()), new byte[] {1});
+            }
+
+            cnt++;
+        }
+
+        assert cnt == 1;
+    }
+
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testClob() throws Exception {
+        ResultSet rs = stmt.executeQuery(SQL);
+
+        int cnt = 0;
+
+        while (rs.next()) {
+            if (cnt == 0) {
+                Clob clob = rs.getClob("clobVal");
+                Assert.assertEquals("str", clob.getSubString(1, (int)clob.length()));
+
+                clob = rs.getClob(24);
+                Assert.assertEquals("str", clob.getSubString(1, (int)clob.length()));
+            }
+
+            cnt++;
+        }
+
+        assert cnt == 1;

Review Comment:
   ```suggestion
           assertTrue(rs.next());
           
           Clob clob = rs.getClob("clobVal");
           Assert.assertEquals("str", clob.getSubString(1, (int)clob.length()));
   
           clob = rs.getClob(24);
           Assert.assertEquals("str", clob.getSubString(1, (int)clob.length()));
           assertFalse(rs.next());
   ```



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-6141 JDBC: add basic support for BLOB and CLOB types [ignite]

Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #11492:
URL: https://github.com/apache/ignite/pull/11492#discussion_r1751798540


##########
modules/clients/src/test/java/org/apache/ignite/jdbc/thin/JdbcThinPreparedStatementSelfTest.java:
##########
@@ -854,6 +858,90 @@ public void testArray() throws Exception {
         assert cnt == 1;
     }
 
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testBlob() throws Exception {
+        stmt = conn.prepareStatement(SQL_PART + " where blobVal is not distinct from ?");
+
+        Blob blob = conn.createBlob();
+
+        blob.setBytes(1, new byte[] {1});
+
+        stmt.setBlob(1, blob);
+
+        ResultSet rs = stmt.executeQuery();
+
+        int cnt = 0;
+
+        while (rs.next()) {
+            if (cnt == 0)
+                assert rs.getInt("id") == 1;
+
+            cnt++;
+        }
+
+        assertEquals(1, cnt);
+
+        stmt.setNull(1, BLOB);
+
+        rs = stmt.executeQuery();
+
+        cnt = 0;
+
+        while (rs.next()) {
+            if (cnt == 0)
+                assert rs.getInt("id") == 2;
+
+            cnt++;
+        }
+
+        assert cnt == 1;

Review Comment:
   ```suggestion
           assertTrue(rs.next());
           assertEquals(2, rs.getInt("id");
           assertFalse(rs.next());
   ```



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-6141 JDBC: add basic support for BLOB and CLOB types [ignite]

Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #11492:
URL: https://github.com/apache/ignite/pull/11492#discussion_r1751798540


##########
modules/clients/src/test/java/org/apache/ignite/jdbc/thin/JdbcThinPreparedStatementSelfTest.java:
##########
@@ -854,6 +858,90 @@ public void testArray() throws Exception {
         assert cnt == 1;
     }
 
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testBlob() throws Exception {
+        stmt = conn.prepareStatement(SQL_PART + " where blobVal is not distinct from ?");
+
+        Blob blob = conn.createBlob();
+
+        blob.setBytes(1, new byte[] {1});
+
+        stmt.setBlob(1, blob);
+
+        ResultSet rs = stmt.executeQuery();
+
+        int cnt = 0;
+
+        while (rs.next()) {
+            if (cnt == 0)
+                assert rs.getInt("id") == 1;
+
+            cnt++;
+        }
+
+        assertEquals(1, cnt);
+
+        stmt.setNull(1, BLOB);
+
+        rs = stmt.executeQuery();
+
+        cnt = 0;
+
+        while (rs.next()) {
+            if (cnt == 0)
+                assert rs.getInt("id") == 2;
+
+            cnt++;
+        }
+
+        assert cnt == 1;

Review Comment:
   ```suggestion
   ```suggestion
           assertTrue(rs.next());
           assertEquals(2, rs.getInt("id");
           assertFalse(rs.next());
   ```
   ```



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-6141 JDBC: add basic support for BLOB and CLOB types [ignite]

Posted by "skorotkov (via GitHub)" <gi...@apache.org>.
skorotkov commented on code in PR #11492:
URL: https://github.com/apache/ignite/pull/11492#discussion_r1743383146


##########
modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/IgniteH2Indexing.java:
##########
@@ -1090,14 +1091,14 @@ private List<? extends FieldsQueryCursor<List<?>>> executeDml(
             if (roEx != null) {
                 throw new IgniteSQLException(
                     "Failed to execute DML statement. Cluster in read-only mode [stmt=" + qryDesc.sql() +
-                    ", params=" + Arrays.deepToString(qryParams.arguments()) + "]",
+                    ", params=" + S.toString(QueryParameters.class, qryParams) + "]",
                     IgniteQueryErrorCode.CLUSTER_READ_ONLY_MODE_ENABLED,
                     e
                 );
             }
 
             throw new IgniteSQLException("Failed to execute DML statement [stmt=" + qryDesc.sql() +
-                ", params=" + Arrays.deepToString(qryParams.arguments()) + "]", e);
+                    ", params=" + S.toString(QueryParameters.class, qryParams) + "]", e);

Review Comment:
   see above



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-6141 JDBC: add basic support for BLOB and CLOB types [ignite]

Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #11492:
URL: https://github.com/apache/ignite/pull/11492#discussion_r1751695572


##########
modules/core/src/main/java/org/apache/ignite/internal/jdbc2/JdbcClob.java:
##########
@@ -0,0 +1,326 @@
+/*
+ * 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.jdbc2;
+
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.Reader;
+import java.io.StringReader;
+import java.io.Writer;
+import java.sql.Clob;
+import java.sql.SQLException;
+import java.sql.SQLFeatureNotSupportedException;
+import org.apache.ignite.internal.util.typedef.internal.U;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+/**
+ * CLOB implementation for Ignite JDBC driver.
+ */
+public class JdbcClob implements Clob {
+    /** CLOB's character sequence. */
+    private String chars;
+
+    /**
+     * @param chars CLOB's character sequence.
+     */
+    public JdbcClob(String chars) {
+        this.chars = chars;
+    }
+
+    /** {@inheritDoc} */
+    @Override public long length() throws SQLException {
+        ensureNotClosed();
+
+        return chars.length();
+    }
+
+    /** {@inheritDoc} */
+    @Override public String getSubString(long pos, int len) throws SQLException {
+        ensureNotClosed();
+
+        long zeroBasedPos = pos - 1;
+
+        if (zeroBasedPos < 0 || len < 0 || zeroBasedPos + len > chars.length())
+            throw new SQLException("Invalid argument. Position should be greater than 0. Length should not be " +
+                "negative. Position + length should be less than CLOB size [pos=" + pos + ", length=" + len + ']');
+
+        return getSubStringInternal(zeroBasedPos, len);
+    }
+
+    /** {@inheritDoc} */
+    @Override public Reader getCharacterStream() throws SQLException {
+        ensureNotClosed();
+
+        return new StringReader(chars);
+    }
+
+    /** {@inheritDoc} */
+    @Override public Reader getCharacterStream(long pos, long len) throws SQLException {
+        return new StringReader(getSubString(pos, (int)len));
+    }
+
+    /** {@inheritDoc} */
+    @Override public InputStream getAsciiStream() throws SQLException {
+        ensureNotClosed();
+
+        // Encode to UTF-8 since Ignite internally stores strings in UTF-8 by default.
+        return new Utf8EncodedStringInputStream(chars);
+    }
+
+    /** {@inheritDoc} */
+    @Override public long position(String searchStr, long start) throws SQLException {
+        ensureNotClosed();
+
+        if (start < 1)
+            throw new SQLException("Invalid argument. Start position should be greater than zero [start=" +
+                start + ']');
+
+        long zeroBasedIdx = positionInternal(searchStr, start - 1);
+
+        return zeroBasedIdx == -1 ? -1 : zeroBasedIdx + 1;
+    }
+
+    /** {@inheritDoc} */
+    @Override public long position(Clob searchStr, long start) throws SQLException {
+        return position(searchStr.getSubString(1, (int)searchStr.length()), start);
+    }
+
+    /** {@inheritDoc} */
+    @Override public int setString(long pos, String str) throws SQLException {
+        ensureNotClosed();
+
+        long zeroBasedPos = pos - 1;
+
+        if (zeroBasedPos < 0 || str == null || zeroBasedPos > chars.length())
+            throw new SQLException("Invalid argument. Position should be greater than zero. " +
+                "Position should not exceed CLOB length+1. Source string should not be null " +
+                "[pos=" + pos + ", str=" + str + ']');
+
+        return setStringInternal(zeroBasedPos, str);
+    }
+
+    /** {@inheritDoc} */
+    @Override public int setString(long pos, String str, int off, int len) throws SQLException {
+        ensureNotClosed();
+
+        long zeroBasedPos = pos - 1;
+
+        if (zeroBasedPos < 0 || str == null || zeroBasedPos > chars.length() || off < 0 || len < 0 || off + len > str.length())
+            throw new SQLException("Invalid argument. Position should be greater than zero. " +
+                "Position should not exceed CLOB length+1. Source string should not be null.  " +
+                "Offset and length shouldn't be negative. Offset + length should not exceed source string length " +
+                "[pos=" + pos + ", str=" + str + ", offset=" + off + ", len=" + len + ']');
+
+        return setStringInternal(zeroBasedPos, str, off, len);
+    }
+
+    /** {@inheritDoc} */
+    @Override public void truncate(long len) throws SQLException {
+        ensureNotClosed();
+
+        if (len < 0 || len > chars.length())
+            throw new SQLException("Invalid argument. Truncation length should not be negative. Truncation length " +
+                "should not exceed data length [len=" + len + ']');
+
+        chars = chars.substring(0, (int)len);
+    }
+
+    /** {@inheritDoc} */
+    @Override public OutputStream setAsciiStream(long pos) throws SQLException {
+        throw new SQLFeatureNotSupportedException();
+    }
+
+    /** {@inheritDoc} */
+    @Override public Writer setCharacterStream(long pos) throws SQLException {
+        throw new SQLFeatureNotSupportedException();
+    }
+
+    /** {@inheritDoc} */
+    @Override public void free() throws SQLException {
+        chars = null;
+    }
+
+    /**
+     * Ensures CLOB hasn't been closed.
+     */
+    private void ensureNotClosed() throws SQLException {
+        if (chars == null)
+            throw new SQLException("Clob instance can't be used after free() has been called.");
+    }
+
+    /**
+     * Internal getSubString implementation the zero-based position parameter.
+     */
+    private String getSubStringInternal(long zeroBasedPos, int len) {
+        return chars.substring((int)zeroBasedPos, (int)zeroBasedPos + len);
+    }
+
+    /**
+     * Internal position implementation with zero-based start parameter.
+     */
+    private long positionInternal(String searchStr, long zeroBasedStart) {
+        return chars.indexOf(searchStr, (int)zeroBasedStart);
+    }
+
+    /**
+     * Internal setString implementation with zero-based position parameter.
+     */
+    private int setStringInternal(long zeroBasedPos, String str) {

Review Comment:
   Minor: Can we have int parameter instead of long? This will reduce cast count.



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-6141 JDBC: add basic support for BLOB and CLOB types [ignite]

Posted by "skorotkov (via GitHub)" <gi...@apache.org>.
skorotkov commented on code in PR #11492:
URL: https://github.com/apache/ignite/pull/11492#discussion_r1743415899


##########
modules/core/src/main/java/org/apache/ignite/internal/jdbc2/JdbcBlob.java:
##########
@@ -54,7 +54,7 @@ public JdbcBlob(byte[] arr) {
     @Override public byte[] getBytes(long pos, int len) throws SQLException {
         ensureNotClosed();
 
-        if (pos < 1 || arr.length - pos < 0 || len < 0)
+        if (pos < 1 || (arr.length - pos < 0 && arr.length > 0) || len < 0)

Review Comment:
   It's needed to support the empty Blobs (blobs backed by an empty array).
   
   Such blobs are returned from the java.sql.Connection::createBlob() which is a standard way to get blob from arbitrary JDBC driver.  It also may be created via the concrete constructor as well like in the unittest below:
   
   ```
           blob = new JdbcBlob(new byte[0]);
           assertEquals(0, blob.getBytes(1, 0).length);
   ```
   This is a correct code according to java.sql.Blob contract.
   
   ***
   
   The corrsponding testcase was added into the file  modules/clients/src/test/java/org/apache/ignite/internal/jdbc2/JdbcBlobTest.java in this PR.



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-6141 JDBC: add basic support for BLOB and CLOB types [ignite]

Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #11492:
URL: https://github.com/apache/ignite/pull/11492#discussion_r1751801377


##########
modules/clients/src/test/java/org/apache/ignite/jdbc/thin/JdbcThinResultSetSelfTest.java:
##########
@@ -599,6 +601,54 @@ public void testArray() throws Exception {
         assert cnt == 1;
     }
 
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testBlob() throws Exception {
+        ResultSet rs = stmt.executeQuery(SQL);
+
+        int cnt = 0;
+
+        while (rs.next()) {
+            if (cnt == 0) {
+                Blob blob = rs.getBlob("blobVal");
+                Assert.assertArrayEquals(blob.getBytes(1, (int)blob.length()), new byte[] {1});
+
+                blob = rs.getBlob(16);
+                Assert.assertArrayEquals(blob.getBytes(1, (int)blob.length()), new byte[] {1});
+            }
+
+            cnt++;
+        }
+
+        assert cnt == 1;

Review Comment:
   ```suggestion
           assertTrue(rs.next());
           Blob blob = rs.getBlob("blobVal");
           Assert.assertArrayEquals(blob.getBytes(1, (int)blob.length()), new byte[] {1});
   
           blob = rs.getBlob(16);
           Assert.assertArrayEquals(blob.getBytes(1, (int)blob.length()), new byte[] {1});
   
           assertFalse(rs.next());
   ```



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-6141 JDBC: add basic support for BLOB and CLOB types [ignite]

Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #11492:
URL: https://github.com/apache/ignite/pull/11492#discussion_r1751697195


##########
modules/core/src/main/java/org/apache/ignite/internal/jdbc2/JdbcClob.java:
##########
@@ -0,0 +1,326 @@
+/*
+ * 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.jdbc2;
+
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.Reader;
+import java.io.StringReader;
+import java.io.Writer;
+import java.sql.Clob;
+import java.sql.SQLException;
+import java.sql.SQLFeatureNotSupportedException;
+import org.apache.ignite.internal.util.typedef.internal.U;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+/**
+ * CLOB implementation for Ignite JDBC driver.
+ */
+public class JdbcClob implements Clob {
+    /** CLOB's character sequence. */
+    private String chars;
+
+    /**
+     * @param chars CLOB's character sequence.
+     */
+    public JdbcClob(String chars) {
+        this.chars = chars;
+    }
+
+    /** {@inheritDoc} */
+    @Override public long length() throws SQLException {
+        ensureNotClosed();
+
+        return chars.length();
+    }
+
+    /** {@inheritDoc} */
+    @Override public String getSubString(long pos, int len) throws SQLException {
+        ensureNotClosed();
+
+        long zeroBasedPos = pos - 1;
+
+        if (zeroBasedPos < 0 || len < 0 || zeroBasedPos + len > chars.length())
+            throw new SQLException("Invalid argument. Position should be greater than 0. Length should not be " +
+                "negative. Position + length should be less than CLOB size [pos=" + pos + ", length=" + len + ']');
+
+        return getSubStringInternal(zeroBasedPos, len);
+    }
+
+    /** {@inheritDoc} */
+    @Override public Reader getCharacterStream() throws SQLException {
+        ensureNotClosed();
+
+        return new StringReader(chars);
+    }
+
+    /** {@inheritDoc} */
+    @Override public Reader getCharacterStream(long pos, long len) throws SQLException {
+        return new StringReader(getSubString(pos, (int)len));
+    }
+
+    /** {@inheritDoc} */
+    @Override public InputStream getAsciiStream() throws SQLException {
+        ensureNotClosed();
+
+        // Encode to UTF-8 since Ignite internally stores strings in UTF-8 by default.
+        return new Utf8EncodedStringInputStream(chars);
+    }
+
+    /** {@inheritDoc} */
+    @Override public long position(String searchStr, long start) throws SQLException {
+        ensureNotClosed();
+
+        if (start < 1)
+            throw new SQLException("Invalid argument. Start position should be greater than zero [start=" +
+                start + ']');
+
+        long zeroBasedIdx = positionInternal(searchStr, start - 1);
+
+        return zeroBasedIdx == -1 ? -1 : zeroBasedIdx + 1;
+    }
+
+    /** {@inheritDoc} */
+    @Override public long position(Clob searchStr, long start) throws SQLException {
+        return position(searchStr.getSubString(1, (int)searchStr.length()), start);
+    }
+
+    /** {@inheritDoc} */
+    @Override public int setString(long pos, String str) throws SQLException {
+        ensureNotClosed();
+
+        long zeroBasedPos = pos - 1;
+
+        if (zeroBasedPos < 0 || str == null || zeroBasedPos > chars.length())
+            throw new SQLException("Invalid argument. Position should be greater than zero. " +
+                "Position should not exceed CLOB length+1. Source string should not be null " +
+                "[pos=" + pos + ", str=" + str + ']');
+
+        return setStringInternal(zeroBasedPos, str);
+    }
+
+    /** {@inheritDoc} */
+    @Override public int setString(long pos, String str, int off, int len) throws SQLException {
+        ensureNotClosed();
+
+        long zeroBasedPos = pos - 1;
+
+        if (zeroBasedPos < 0 || str == null || zeroBasedPos > chars.length() || off < 0 || len < 0 || off + len > str.length())
+            throw new SQLException("Invalid argument. Position should be greater than zero. " +
+                "Position should not exceed CLOB length+1. Source string should not be null.  " +
+                "Offset and length shouldn't be negative. Offset + length should not exceed source string length " +
+                "[pos=" + pos + ", str=" + str + ", offset=" + off + ", len=" + len + ']');
+
+        return setStringInternal(zeroBasedPos, str, off, len);
+    }
+
+    /** {@inheritDoc} */
+    @Override public void truncate(long len) throws SQLException {
+        ensureNotClosed();
+
+        if (len < 0 || len > chars.length())
+            throw new SQLException("Invalid argument. Truncation length should not be negative. Truncation length " +
+                "should not exceed data length [len=" + len + ']');
+
+        chars = chars.substring(0, (int)len);
+    }
+
+    /** {@inheritDoc} */
+    @Override public OutputStream setAsciiStream(long pos) throws SQLException {
+        throw new SQLFeatureNotSupportedException();
+    }
+
+    /** {@inheritDoc} */
+    @Override public Writer setCharacterStream(long pos) throws SQLException {
+        throw new SQLFeatureNotSupportedException();
+    }
+
+    /** {@inheritDoc} */
+    @Override public void free() throws SQLException {
+        chars = null;
+    }
+
+    /**
+     * Ensures CLOB hasn't been closed.
+     */
+    private void ensureNotClosed() throws SQLException {
+        if (chars == null)
+            throw new SQLException("Clob instance can't be used after free() has been called.");
+    }
+
+    /**
+     * Internal getSubString implementation the zero-based position parameter.
+     */
+    private String getSubStringInternal(long zeroBasedPos, int len) {
+        return chars.substring((int)zeroBasedPos, (int)zeroBasedPos + len);
+    }
+
+    /**
+     * Internal position implementation with zero-based start parameter.
+     */
+    private long positionInternal(String searchStr, long zeroBasedStart) {
+        return chars.indexOf(searchStr, (int)zeroBasedStart);
+    }
+
+    /**
+     * Internal setString implementation with zero-based position parameter.
+     */
+    private int setStringInternal(long zeroBasedPos, String str) {
+        StringBuilder strBuilder = new StringBuilder(chars);
+
+        // Ensure string buffer capacity
+        if (zeroBasedPos + str.length() > chars.length())
+            strBuilder.setLength((int)zeroBasedPos + str.length());
+
+        strBuilder.replace((int)zeroBasedPos, (int)zeroBasedPos + str.length(), str);
+
+        chars = strBuilder.toString();
+
+        return str.length();
+    }
+
+    /**
+     * Internal setString implementation with zero-based position parameter.
+     */
+    private int setStringInternal(long zeroBasedPos, String str, int off, int len) {

Review Comment:
   Minor: Can we have int parameter instead of long? This will reduce cast count.



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-6141 JDBC thin: support BLOB and CLOB types [ignite]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #11492:
URL: https://github.com/apache/ignite/pull/11492#issuecomment-2320820658

   ## [![Quality Gate Failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-failed-20px.png 'Quality Gate Failed')](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=11492) **Quality Gate failed**  
   Failed conditions  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/failed-16px.png '') [5 New Code Smells](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=11492) (required ≤ 1)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=11492)
   
   ##   
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/light_bulb-16px.png '') Catch issues before they fail your Quality Gate with our IDE extension ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/sonarlint-16px.png '') [SonarLint](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=pull-request)
   
   


-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-6141 JDBC: add basic support for BLOB and CLOB types [ignite]

Posted by "skorotkov (via GitHub)" <gi...@apache.org>.
skorotkov commented on code in PR #11492:
URL: https://github.com/apache/ignite/pull/11492#discussion_r1752203676


##########
modules/clients/src/test/java/org/apache/ignite/internal/jdbc2/JdbcClobTest.java:
##########
@@ -0,0 +1,448 @@
+/*
+ * 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.jdbc2;
+
+import java.io.InputStream;
+import java.io.Reader;
+import java.sql.Clob;
+import java.sql.SQLException;
+import java.sql.SQLFeatureNotSupportedException;
+import org.apache.commons.io.IOUtils;
+import org.junit.Assert;
+import org.junit.Test;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.apache.ignite.testframework.GridTestUtils.assertThrows;
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Test for JDBC CLOB.
+ */
+public class JdbcClobTest {
+    /** */
+    static final String ERROR_CLOB_FREE = "Clob instance can't be used after free() has been called.";
+
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testLength() throws Exception {
+        JdbcClob clob = new JdbcClob("1234567890");
+
+        assertEquals(10, clob.length());
+
+        clob.free();
+        assertThrows(null, clob::length, SQLException.class, ERROR_CLOB_FREE);
+    }
+
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testGetSubString() throws Exception {
+        JdbcClob clob = new JdbcClob("1234567890");
+
+        assertThrows(null, () -> clob.getSubString(-1, 1), SQLException.class, null);
+
+        assertThrows(null, () -> clob.getSubString(0, 1), SQLException.class, null);
+
+        assertThrows(null, () -> clob.getSubString(1, -1), SQLException.class, null);
+
+        assertThrows(null, () -> clob.getSubString(1, 11), SQLException.class, null);
+
+        assertEquals("", clob.getSubString(3, 0));
+
+        assertEquals("1", clob.getSubString(1, 1));
+
+        assertEquals("0", clob.getSubString(10, 1));
+
+        assertEquals("12345", clob.getSubString(1, 5));
+
+        assertEquals("34567", clob.getSubString(3, 5));
+
+        assertEquals("567890", clob.getSubString(5, 6));
+
+        assertEquals("1234567890", clob.getSubString(1, 10));
+
+        clob.free();
+        assertThrows(null, () -> clob.getSubString(1, 10), SQLException.class, ERROR_CLOB_FREE);
+    }
+
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testGetCharacterStream() throws Exception {
+        JdbcClob clob = new JdbcClob("1234567890");
+
+        Reader cStream = clob.getCharacterStream();
+        String res = IOUtils.toString(cStream);
+        assertEquals("1234567890", res);
+
+        clob.free();
+        assertThrows(null, () -> clob.getCharacterStream(), SQLException.class, ERROR_CLOB_FREE);
+    }
+
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testGetCharacterStreamWithParams() throws Exception {
+        JdbcClob clob = new JdbcClob("1234567890");
+
+        assertThrows(null, () -> clob.getCharacterStream(-1, 1), SQLException.class, null);
+
+        assertThrows(null, () -> clob.getCharacterStream(0, 1), SQLException.class, null);
+
+        assertThrows(null, () -> clob.getCharacterStream(1, -1), SQLException.class, null);
+
+        assertThrows(null, () -> clob.getCharacterStream(1, 11), SQLException.class, null);
+
+        Reader cStream = clob.getCharacterStream(1, 10);
+        String res = IOUtils.toString(cStream);
+        assertEquals("1234567890", res);
+
+        cStream = clob.getCharacterStream(1, 1);
+        res = IOUtils.toString(cStream);
+        assertEquals("1", res);
+
+        cStream = clob.getCharacterStream(10, 1);
+        res = IOUtils.toString(cStream);
+        assertEquals("0", res);
+
+        cStream = clob.getCharacterStream(3, 5);
+        res = IOUtils.toString(cStream);
+        assertEquals("34567", res);
+
+        cStream = clob.getCharacterStream(3, 0);
+        res = IOUtils.toString(cStream);
+        assertEquals("", res);
+
+        clob.free();
+        assertThrows(null, () -> clob.getCharacterStream(1, 1), SQLException.class, ERROR_CLOB_FREE);
+    }
+
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testGetAsciiStream() throws Exception {
+        JdbcClob clob = new JdbcClob("1234567890");
+        byte[] bytes = IOUtils.toByteArray(clob.getAsciiStream());
+        Assert.assertArrayEquals("1234567890".getBytes(UTF_8), bytes);
+
+        clob.free();
+        assertThrows(null, clob::getAsciiStream, SQLException.class, ERROR_CLOB_FREE);
+
+        Clob emptyClob = new JdbcClob("");
+        bytes = IOUtils.toByteArray(emptyClob.getAsciiStream());
+        Assert.assertArrayEquals("".getBytes(UTF_8), bytes);
+    }
+
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testGetAsciiStreamForNonAsciiDataBufferedRead() throws Exception {
+        StringBuilder sb = new StringBuilder();
+        for (int i = 0; i < 10000; i++) {
+            sb.append("aa©😀");
+        }
+
+        Clob clob = new JdbcClob(sb.toString());
+
+        InputStream stream = clob.getAsciiStream();
+
+        assertThrows(null, () -> stream.read(null, 0, 1), NullPointerException.class, null);
+
+        assertThrows(null, () -> stream.read(new byte[10], -1, 5), IndexOutOfBoundsException.class, null);
+
+        assertThrows(null, () -> stream.read(new byte[10], 5, -1), IndexOutOfBoundsException.class, null);
+
+        assertThrows(null, () -> stream.read(new byte[10], 11, 1), IndexOutOfBoundsException.class, null);
+
+        assertThrows(null, () -> stream.read(new byte[10], 5, 6), IndexOutOfBoundsException.class, null);
+
+        assertEquals(0, stream.read(new byte[10], 5, 0));
+
+        byte[] bytes = IOUtils.toByteArray(stream);
+
+        String reencoded = new String(bytes, UTF_8);
+
+        assertEquals(clob.getSubString(1, (int)clob.length()), reencoded);
+    }
+
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testGetAsciiStreamForNonAsciiDataReadByByte() throws Exception {
+        StringBuilder sb = new StringBuilder();
+        for (int i = 0; i < 10000; i++) {

Review Comment:
   The idea was to cover all variants in the JdbcClob.Utf8EncodedStringInputStream.encodeNextChunk() implementaion.
   
   I reduce this to 3277 in one of two testcases (the minimum which produces the malformed surrogate).
   
   And change to 10 in another one.



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-6141 JDBC: add basic support for BLOB and CLOB types [ignite]

Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov merged PR #11492:
URL: https://github.com/apache/ignite/pull/11492


-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-6141 JDBC: add basic support for BLOB and CLOB types [ignite]

Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #11492:
URL: https://github.com/apache/ignite/pull/11492#discussion_r1751781915


##########
modules/clients/src/test/java/org/apache/ignite/internal/jdbc2/JdbcClobTest.java:
##########
@@ -0,0 +1,448 @@
+/*
+ * 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.jdbc2;
+
+import java.io.InputStream;
+import java.io.Reader;
+import java.sql.Clob;
+import java.sql.SQLException;
+import java.sql.SQLFeatureNotSupportedException;
+import org.apache.commons.io.IOUtils;
+import org.junit.Assert;
+import org.junit.Test;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.apache.ignite.testframework.GridTestUtils.assertThrows;
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Test for JDBC CLOB.
+ */
+public class JdbcClobTest {
+    /** */
+    static final String ERROR_CLOB_FREE = "Clob instance can't be used after free() has been called.";
+
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testLength() throws Exception {
+        JdbcClob clob = new JdbcClob("1234567890");
+
+        assertEquals(10, clob.length());
+
+        clob.free();
+        assertThrows(null, clob::length, SQLException.class, ERROR_CLOB_FREE);
+    }
+
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testGetSubString() throws Exception {
+        JdbcClob clob = new JdbcClob("1234567890");
+
+        assertThrows(null, () -> clob.getSubString(-1, 1), SQLException.class, null);
+
+        assertThrows(null, () -> clob.getSubString(0, 1), SQLException.class, null);
+
+        assertThrows(null, () -> clob.getSubString(1, -1), SQLException.class, null);
+
+        assertThrows(null, () -> clob.getSubString(1, 11), SQLException.class, null);
+
+        assertEquals("", clob.getSubString(3, 0));
+
+        assertEquals("1", clob.getSubString(1, 1));
+
+        assertEquals("0", clob.getSubString(10, 1));
+
+        assertEquals("12345", clob.getSubString(1, 5));
+
+        assertEquals("34567", clob.getSubString(3, 5));
+
+        assertEquals("567890", clob.getSubString(5, 6));
+
+        assertEquals("1234567890", clob.getSubString(1, 10));
+
+        clob.free();
+        assertThrows(null, () -> clob.getSubString(1, 10), SQLException.class, ERROR_CLOB_FREE);
+    }
+
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testGetCharacterStream() throws Exception {
+        JdbcClob clob = new JdbcClob("1234567890");
+
+        Reader cStream = clob.getCharacterStream();
+        String res = IOUtils.toString(cStream);
+        assertEquals("1234567890", res);
+
+        clob.free();
+        assertThrows(null, () -> clob.getCharacterStream(), SQLException.class, ERROR_CLOB_FREE);
+    }
+
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testGetCharacterStreamWithParams() throws Exception {
+        JdbcClob clob = new JdbcClob("1234567890");
+
+        assertThrows(null, () -> clob.getCharacterStream(-1, 1), SQLException.class, null);
+
+        assertThrows(null, () -> clob.getCharacterStream(0, 1), SQLException.class, null);
+
+        assertThrows(null, () -> clob.getCharacterStream(1, -1), SQLException.class, null);
+
+        assertThrows(null, () -> clob.getCharacterStream(1, 11), SQLException.class, null);
+
+        Reader cStream = clob.getCharacterStream(1, 10);
+        String res = IOUtils.toString(cStream);
+        assertEquals("1234567890", res);
+
+        cStream = clob.getCharacterStream(1, 1);
+        res = IOUtils.toString(cStream);
+        assertEquals("1", res);
+
+        cStream = clob.getCharacterStream(10, 1);
+        res = IOUtils.toString(cStream);
+        assertEquals("0", res);
+
+        cStream = clob.getCharacterStream(3, 5);
+        res = IOUtils.toString(cStream);
+        assertEquals("34567", res);
+
+        cStream = clob.getCharacterStream(3, 0);
+        res = IOUtils.toString(cStream);
+        assertEquals("", res);
+
+        clob.free();
+        assertThrows(null, () -> clob.getCharacterStream(1, 1), SQLException.class, ERROR_CLOB_FREE);
+    }
+
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testGetAsciiStream() throws Exception {
+        JdbcClob clob = new JdbcClob("1234567890");
+        byte[] bytes = IOUtils.toByteArray(clob.getAsciiStream());
+        Assert.assertArrayEquals("1234567890".getBytes(UTF_8), bytes);
+
+        clob.free();
+        assertThrows(null, clob::getAsciiStream, SQLException.class, ERROR_CLOB_FREE);
+
+        Clob emptyClob = new JdbcClob("");
+        bytes = IOUtils.toByteArray(emptyClob.getAsciiStream());
+        Assert.assertArrayEquals("".getBytes(UTF_8), bytes);
+    }
+
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testGetAsciiStreamForNonAsciiDataBufferedRead() throws Exception {
+        StringBuilder sb = new StringBuilder();
+        for (int i = 0; i < 10000; i++) {
+            sb.append("aa©😀");
+        }
+
+        Clob clob = new JdbcClob(sb.toString());
+
+        InputStream stream = clob.getAsciiStream();
+
+        assertThrows(null, () -> stream.read(null, 0, 1), NullPointerException.class, null);
+
+        assertThrows(null, () -> stream.read(new byte[10], -1, 5), IndexOutOfBoundsException.class, null);
+
+        assertThrows(null, () -> stream.read(new byte[10], 5, -1), IndexOutOfBoundsException.class, null);
+
+        assertThrows(null, () -> stream.read(new byte[10], 11, 1), IndexOutOfBoundsException.class, null);
+
+        assertThrows(null, () -> stream.read(new byte[10], 5, 6), IndexOutOfBoundsException.class, null);
+
+        assertEquals(0, stream.read(new byte[10], 5, 0));
+
+        byte[] bytes = IOUtils.toByteArray(stream);
+
+        String reencoded = new String(bytes, UTF_8);
+
+        assertEquals(clob.getSubString(1, (int)clob.length()), reencoded);
+    }
+
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testGetAsciiStreamForNonAsciiDataReadByByte() throws Exception {
+        StringBuilder sb = new StringBuilder();
+        for (int i = 0; i < 10000; i++) {

Review Comment:
   do we really need 10000 copies of the string?
   Let's help CI and keep 10 copies?



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-6141 JDBC: add basic support for BLOB and CLOB types [ignite]

Posted by "skorotkov (via GitHub)" <gi...@apache.org>.
skorotkov commented on code in PR #11492:
URL: https://github.com/apache/ignite/pull/11492#discussion_r1747025095


##########
modules/core/src/main/java/org/apache/ignite/internal/jdbc2/JdbcClob.java:
##########
@@ -0,0 +1,292 @@
+/*
+ * 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.jdbc2;
+
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.Reader;
+import java.io.StringReader;
+import java.io.Writer;
+import java.sql.Clob;
+import java.sql.SQLException;
+import java.sql.SQLFeatureNotSupportedException;
+import org.apache.ignite.internal.util.typedef.internal.U;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+/**
+ * CLOB implementation for Ignite JDBC driver.
+ */
+public class JdbcClob implements Clob {

Review Comment:
   I think it's not worth doing so for current basic and naive Clob support.
   
   Say **postgres** jdbc driver just doesn't support any methods to change the `Clob` object at all. The **mysql** jdbc driver does almost the same as we do here - it creates streams around the underlying string. **Oracle** possibly does something else but it has a real Clob support on protocol and storage level, which ignite doesn't.  In ignite Clob for now is just a thin wrapper around the Strings.
   
   General JDBC application wouldn't create a stream to **read** data from Clob from ResutSet and **change** it at the same time.  Especially considering the "JDBC Thin Driver is not thread safe".
   
   As far as the `char[]` implementation it would increase the memory usage in the "read" case. It would require the creation of extra char[] array in the ResultSet::getClob.  With current implementation just a string reference is passed and no extra memory is needed (especially if the get*Stream() APIs are used by the application).



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-6141 JDBC: add basic support for BLOB and CLOB types [ignite]

Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #11492:
URL: https://github.com/apache/ignite/pull/11492#discussion_r1751699325


##########
modules/core/src/main/java/org/apache/ignite/internal/jdbc2/JdbcClob.java:
##########
@@ -0,0 +1,326 @@
+/*
+ * 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.jdbc2;
+
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.Reader;
+import java.io.StringReader;
+import java.io.Writer;
+import java.sql.Clob;
+import java.sql.SQLException;
+import java.sql.SQLFeatureNotSupportedException;
+import org.apache.ignite.internal.util.typedef.internal.U;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+/**
+ * CLOB implementation for Ignite JDBC driver.
+ */
+public class JdbcClob implements Clob {
+    /** CLOB's character sequence. */
+    private String chars;
+
+    /**
+     * @param chars CLOB's character sequence.
+     */
+    public JdbcClob(String chars) {
+        this.chars = chars;
+    }
+
+    /** {@inheritDoc} */
+    @Override public long length() throws SQLException {
+        ensureNotClosed();
+
+        return chars.length();
+    }
+
+    /** {@inheritDoc} */
+    @Override public String getSubString(long pos, int len) throws SQLException {
+        ensureNotClosed();
+
+        long zeroBasedPos = pos - 1;
+
+        if (zeroBasedPos < 0 || len < 0 || zeroBasedPos + len > chars.length())
+            throw new SQLException("Invalid argument. Position should be greater than 0. Length should not be " +
+                "negative. Position + length should be less than CLOB size [pos=" + pos + ", length=" + len + ']');
+
+        return getSubStringInternal(zeroBasedPos, len);
+    }
+
+    /** {@inheritDoc} */
+    @Override public Reader getCharacterStream() throws SQLException {
+        ensureNotClosed();
+
+        return new StringReader(chars);
+    }
+
+    /** {@inheritDoc} */
+    @Override public Reader getCharacterStream(long pos, long len) throws SQLException {
+        return new StringReader(getSubString(pos, (int)len));
+    }
+
+    /** {@inheritDoc} */
+    @Override public InputStream getAsciiStream() throws SQLException {
+        ensureNotClosed();
+
+        // Encode to UTF-8 since Ignite internally stores strings in UTF-8 by default.
+        return new Utf8EncodedStringInputStream(chars);
+    }
+
+    /** {@inheritDoc} */
+    @Override public long position(String searchStr, long start) throws SQLException {
+        ensureNotClosed();
+
+        if (start < 1)
+            throw new SQLException("Invalid argument. Start position should be greater than zero [start=" +
+                start + ']');
+
+        long zeroBasedIdx = positionInternal(searchStr, start - 1);
+
+        return zeroBasedIdx == -1 ? -1 : zeroBasedIdx + 1;
+    }
+
+    /** {@inheritDoc} */
+    @Override public long position(Clob searchStr, long start) throws SQLException {
+        return position(searchStr.getSubString(1, (int)searchStr.length()), start);
+    }
+
+    /** {@inheritDoc} */
+    @Override public int setString(long pos, String str) throws SQLException {
+        ensureNotClosed();
+
+        long zeroBasedPos = pos - 1;
+
+        if (zeroBasedPos < 0 || str == null || zeroBasedPos > chars.length())
+            throw new SQLException("Invalid argument. Position should be greater than zero. " +
+                "Position should not exceed CLOB length+1. Source string should not be null " +
+                "[pos=" + pos + ", str=" + str + ']');
+
+        return setStringInternal(zeroBasedPos, str);
+    }
+
+    /** {@inheritDoc} */
+    @Override public int setString(long pos, String str, int off, int len) throws SQLException {
+        ensureNotClosed();
+
+        long zeroBasedPos = pos - 1;
+
+        if (zeroBasedPos < 0 || str == null || zeroBasedPos > chars.length() || off < 0 || len < 0 || off + len > str.length())
+            throw new SQLException("Invalid argument. Position should be greater than zero. " +
+                "Position should not exceed CLOB length+1. Source string should not be null.  " +
+                "Offset and length shouldn't be negative. Offset + length should not exceed source string length " +
+                "[pos=" + pos + ", str=" + str + ", offset=" + off + ", len=" + len + ']');
+
+        return setStringInternal(zeroBasedPos, str, off, len);
+    }
+
+    /** {@inheritDoc} */
+    @Override public void truncate(long len) throws SQLException {
+        ensureNotClosed();
+
+        if (len < 0 || len > chars.length())
+            throw new SQLException("Invalid argument. Truncation length should not be negative. Truncation length " +
+                "should not exceed data length [len=" + len + ']');
+
+        chars = chars.substring(0, (int)len);
+    }
+
+    /** {@inheritDoc} */
+    @Override public OutputStream setAsciiStream(long pos) throws SQLException {
+        throw new SQLFeatureNotSupportedException();
+    }
+
+    /** {@inheritDoc} */
+    @Override public Writer setCharacterStream(long pos) throws SQLException {
+        throw new SQLFeatureNotSupportedException();
+    }
+
+    /** {@inheritDoc} */
+    @Override public void free() throws SQLException {
+        chars = null;
+    }
+
+    /**
+     * Ensures CLOB hasn't been closed.
+     */
+    private void ensureNotClosed() throws SQLException {
+        if (chars == null)
+            throw new SQLException("Clob instance can't be used after free() has been called.");
+    }
+
+    /**
+     * Internal getSubString implementation the zero-based position parameter.
+     */
+    private String getSubStringInternal(long zeroBasedPos, int len) {
+        return chars.substring((int)zeroBasedPos, (int)zeroBasedPos + len);
+    }
+
+    /**
+     * Internal position implementation with zero-based start parameter.
+     */
+    private long positionInternal(String searchStr, long zeroBasedStart) {
+        return chars.indexOf(searchStr, (int)zeroBasedStart);
+    }
+
+    /**
+     * Internal setString implementation with zero-based position parameter.
+     */
+    private int setStringInternal(long zeroBasedPos, String str) {
+        StringBuilder strBuilder = new StringBuilder(chars);
+
+        // Ensure string buffer capacity
+        if (zeroBasedPos + str.length() > chars.length())
+            strBuilder.setLength((int)zeroBasedPos + str.length());
+
+        strBuilder.replace((int)zeroBasedPos, (int)zeroBasedPos + str.length(), str);
+
+        chars = strBuilder.toString();
+
+        return str.length();
+    }
+
+    /**
+     * Internal setString implementation with zero-based position parameter.
+     */
+    private int setStringInternal(long zeroBasedPos, String str, int off, int len) {
+        StringBuilder strBuilder = new StringBuilder(chars);
+
+        // Ensure string buffer capacity
+        if (zeroBasedPos + str.length() > chars.length())
+            strBuilder.setLength((int)zeroBasedPos + str.length());
+
+        String replaceStr = str.substring(off, off + len);
+        strBuilder.replace((int)zeroBasedPos, (int)zeroBasedPos + replaceStr.length(), replaceStr);
+
+        chars = strBuilder.toString();
+
+        return replaceStr.length();
+    }
+
+    /**
+     * Input stream which encodes the given string to UTF-8.
+     * To save memory for large strings it does it by chunks.
+     */
+    private static class Utf8EncodedStringInputStream extends InputStream {
+        /** String to encode. */
+        private final String chars;
+
+        /** String length. */
+        private final int length;
+
+        /** Start index of the next chunk (substring) to be encoded. */
+        private int charsPos;
+
+        /** Default chunk size. */
+        private static final int DEFAULT_CHUNK_SIZE = 8192;
+
+        /** Buffer containing the current chunk encoding. */
+        private byte[] buf;
+
+        /** Current position in the buffer - index of the next byte to be read from the input stream. */
+        private int bufPos;
+
+        /**
+         * @param chars String to be encoded.
+         */
+        Utf8EncodedStringInputStream(String chars) {
+            this.chars = chars;
+
+            length = chars.length();
+            charsPos = 0;
+        }
+
+        /** {@inheritDoc} */
+        @Override public synchronized int read() {
+            if (buf == null || buf.length == 0 || bufPos >= buf.length) {
+                if (charsPos >= length)
+                    return -1;
+
+                bufPos = 0;
+
+                encodeNextChunk();
+            }
+
+            return buf[bufPos++] & 0xFF;
+        }
+
+        /** {@inheritDoc} */
+        @Override public synchronized int read(byte b[], int off, int len) {
+            if (b == null)
+                throw new NullPointerException();
+
+            if (off < 0 || len < 0 || len > b.length - off)
+                throw new IndexOutOfBoundsException(String.format("Range [%s, %<s + %s) out of bounds for length %s", off, len, b.length));
+
+            if (len == 0)
+                return 0;
+
+            int i = 0;
+
+            while (i < len) {
+                if (buf == null || buf.length == 0 || bufPos >= buf.length) {
+                    if (charsPos >= length)
+                        return i > 0 ? i : -1;
+
+                    bufPos = 0;
+
+                    encodeNextChunk();
+                }
+
+                int encoded_chunk_size = Math.min(len - i, buf.length - bufPos);

Review Comment:
   encoded_chunk_size -> encodedChunkSize



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-6141 JDBC: add basic support for BLOB and CLOB types [ignite]

Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #11492:
URL: https://github.com/apache/ignite/pull/11492#discussion_r1751800183


##########
modules/clients/src/test/java/org/apache/ignite/jdbc/thin/JdbcThinPreparedStatementSelfTest.java:
##########
@@ -854,6 +858,90 @@ public void testArray() throws Exception {
         assert cnt == 1;
     }
 
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testBlob() throws Exception {
+        stmt = conn.prepareStatement(SQL_PART + " where blobVal is not distinct from ?");
+
+        Blob blob = conn.createBlob();
+
+        blob.setBytes(1, new byte[] {1});
+
+        stmt.setBlob(1, blob);
+
+        ResultSet rs = stmt.executeQuery();
+
+        int cnt = 0;
+
+        while (rs.next()) {
+            if (cnt == 0)
+                assert rs.getInt("id") == 1;
+
+            cnt++;
+        }
+
+        assertEquals(1, cnt);
+
+        stmt.setNull(1, BLOB);
+
+        rs = stmt.executeQuery();
+
+        cnt = 0;
+
+        while (rs.next()) {
+            if (cnt == 0)
+                assert rs.getInt("id") == 2;
+
+            cnt++;
+        }
+
+        assert cnt == 1;
+    }
+
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testClob() throws Exception {
+        stmt = conn.prepareStatement(SQL_PART + " where clobVal is not distinct from ?");
+
+        Clob clob = conn.createClob();
+
+        clob.setString(1, "large str");
+
+        stmt.setClob(1, clob);
+
+        ResultSet rs = stmt.executeQuery();
+
+        int cnt = 0;
+
+        while (rs.next()) {
+            if (cnt == 0)
+                assert rs.getInt("id") == 1;
+
+            cnt++;
+        }
+
+        assertEquals(1, cnt);
+
+        stmt.setNull(1, CLOB);
+
+        rs = stmt.executeQuery();
+
+        cnt = 0;
+
+        while (rs.next()) {
+            if (cnt == 0)
+                assert rs.getInt("id") == 2;
+
+            cnt++;
+        }
+
+        assert cnt == 1;

Review Comment:
   ```suggestion
           assertTrue(rs.next());
           assertEquals(2, rs.getInt("id");
           assertFalse(rs.next());
   ```



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-6141 JDBC: add basic support for BLOB and CLOB types [ignite]

Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #11492:
URL: https://github.com/apache/ignite/pull/11492#discussion_r1751802725


##########
modules/clients/src/test/java/org/apache/ignite/jdbc/thin/JdbcThinResultSetSelfTest.java:
##########
@@ -599,6 +601,54 @@ public void testArray() throws Exception {
         assert cnt == 1;
     }
 
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testBlob() throws Exception {
+        ResultSet rs = stmt.executeQuery(SQL);
+
+        int cnt = 0;
+
+        while (rs.next()) {
+            if (cnt == 0) {
+                Blob blob = rs.getBlob("blobVal");
+                Assert.assertArrayEquals(blob.getBytes(1, (int)blob.length()), new byte[] {1});
+
+                blob = rs.getBlob(16);
+                Assert.assertArrayEquals(blob.getBytes(1, (int)blob.length()), new byte[] {1});
+            }
+
+            cnt++;
+        }
+
+        assert cnt == 1;
+    }
+
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testClob() throws Exception {
+        ResultSet rs = stmt.executeQuery(SQL);
+
+        int cnt = 0;
+
+        while (rs.next()) {
+            if (cnt == 0) {
+                Clob clob = rs.getClob("clobVal");
+                Assert.assertEquals("str", clob.getSubString(1, (int)clob.length()));
+
+                clob = rs.getClob(17);
+                Assert.assertEquals("str", clob.getSubString(1, (int)clob.length()));
+            }
+
+            cnt++;
+        }
+
+        assert cnt == 1;

Review Comment:
   ```suggestion
           assertTrue(rs.next());
   
           Clob clob = rs.getClob("clobVal");
           Assert.assertEquals("str", clob.getSubString(1, (int)clob.length()));
   
           clob = rs.getClob(17);
           Assert.assertEquals("str", clob.getSubString(1, (int)clob.length()));
           assertFalse(rs.next());
   ```



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-6141 JDBC: add basic support for BLOB and CLOB types [ignite]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #11492:
URL: https://github.com/apache/ignite/pull/11492#issuecomment-2341447776

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=11492) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 New issues](https://sonarcloud.io/project/issues?id=apache_ignite&pullRequest=11492&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_ignite&pullRequest=11492&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_ignite&pullRequest=11492&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_ignite&pullRequest=11492&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [1.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_ignite&pullRequest=11492&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=11492)
   
   


-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-6141 JDBC: add basic support for BLOB and CLOB types [ignite]

Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #11492:
URL: https://github.com/apache/ignite/pull/11492#discussion_r1751798128


##########
modules/clients/src/test/java/org/apache/ignite/jdbc/thin/JdbcThinPreparedStatementSelfTest.java:
##########
@@ -854,6 +858,90 @@ public void testArray() throws Exception {
         assert cnt == 1;
     }
 
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testBlob() throws Exception {
+        stmt = conn.prepareStatement(SQL_PART + " where blobVal is not distinct from ?");
+
+        Blob blob = conn.createBlob();
+
+        blob.setBytes(1, new byte[] {1});
+
+        stmt.setBlob(1, blob);
+
+        ResultSet rs = stmt.executeQuery();
+
+        int cnt = 0;
+
+        while (rs.next()) {
+            if (cnt == 0)
+                assert rs.getInt("id") == 1;
+
+            cnt++;
+        }
+
+        assertEquals(1, cnt);

Review Comment:
   ```suggestion
           assertTrue(rs.next());
           assertEquals(1, rs.getInt("id");
           assertFalse(rs.next());
   ```



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-6141 JDBC: add basic support for BLOB and CLOB types [ignite]

Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #11492:
URL: https://github.com/apache/ignite/pull/11492#discussion_r1751793191


##########
modules/clients/src/test/java/org/apache/ignite/internal/jdbc2/JdbcPreparedStatementSelfTest.java:
##########
@@ -582,7 +586,49 @@ public void testBlob() throws Exception {
 
         assertEquals(1, cnt);
 
-        stmt.setNull(1, BINARY);
+        stmt.setNull(1, BLOB);
+
+        rs = stmt.executeQuery();
+
+        cnt = 0;
+
+        while (rs.next()) {
+            if (cnt == 0)
+                assert rs.getInt("id") == 2;
+
+            cnt++;
+        }
+
+        assert cnt == 1;
+    }
+
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testClob() throws Exception {
+        stmt = conn.prepareStatement("select * from TestObject where clobVal is not distinct from ?");
+
+        Clob clob = conn.createClob();
+
+        clob.setString(1, "large str");
+
+        stmt.setClob(1, clob);
+
+        ResultSet rs = stmt.executeQuery();
+
+        int cnt = 0;
+
+        while (rs.next()) {
+            if (cnt == 0)
+                assert rs.getInt("id") == 1;
+
+            cnt++;
+        }
+
+        assertEquals(1, cnt);

Review Comment:
   ```suggestion
           assertTrue(rs.next());
           assertFalse(rs.next());
   ```



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-6141 JDBC: add basic support for BLOB and CLOB types [ignite]

Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #11492:
URL: https://github.com/apache/ignite/pull/11492#discussion_r1751784979


##########
modules/clients/src/test/java/org/apache/ignite/internal/jdbc2/JdbcPreparedStatementSelfTest.java:
##########
@@ -582,7 +586,49 @@ public void testBlob() throws Exception {
 
         assertEquals(1, cnt);
 
-        stmt.setNull(1, BINARY);
+        stmt.setNull(1, BLOB);
+
+        rs = stmt.executeQuery();
+
+        cnt = 0;
+
+        while (rs.next()) {
+            if (cnt == 0)
+                assert rs.getInt("id") == 2;
+
+            cnt++;
+        }

Review Comment:
   ```suggestion
           assertTrue(rs.next());
           assertEquals(2, rs.getInt("id"));
           assertFalse(rs.next());
   ```



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-6141 JDBC: add basic support for BLOB and CLOB types [ignite]

Posted by "skorotkov (via GitHub)" <gi...@apache.org>.
skorotkov commented on code in PR #11492:
URL: https://github.com/apache/ignite/pull/11492#discussion_r1746997359


##########
modules/clients/src/test/java/org/apache/ignite/internal/jdbc2/JdbcClobTest.java:
##########
@@ -0,0 +1,744 @@
+/*
+ * 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.jdbc2;
+
+import java.io.InputStream;
+import java.io.Reader;
+import java.sql.Clob;
+import java.sql.SQLException;
+import java.sql.SQLFeatureNotSupportedException;
+import org.apache.commons.io.IOUtils;
+import org.junit.Assert;
+import org.junit.Test;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+/**
+ * Test for JDBC CLOB.
+ */
+public class JdbcClobTest {
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testLength() throws Exception {
+        JdbcClob clob = new JdbcClob("1234567890");
+
+        assertEquals(10, clob.length());
+
+        clob.free();
+
+        try {

Review Comment:
   done
   



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-6141 JDBC: add basic support for BLOB and CLOB types [ignite]

Posted by "skorotkov (via GitHub)" <gi...@apache.org>.
skorotkov commented on code in PR #11492:
URL: https://github.com/apache/ignite/pull/11492#discussion_r1747025095


##########
modules/core/src/main/java/org/apache/ignite/internal/jdbc2/JdbcClob.java:
##########
@@ -0,0 +1,292 @@
+/*
+ * 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.jdbc2;
+
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.Reader;
+import java.io.StringReader;
+import java.io.Writer;
+import java.sql.Clob;
+import java.sql.SQLException;
+import java.sql.SQLFeatureNotSupportedException;
+import org.apache.ignite.internal.util.typedef.internal.U;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+/**
+ * CLOB implementation for Ignite JDBC driver.
+ */
+public class JdbcClob implements Clob {

Review Comment:
   I think it's not worth doing so for current basic and naive Clob support.
   
   Say postgres jdbc driver just doesn't support any methods to change the `Clob` object at all. The mysql jdbc driver does almost the same as we do here - it creates streams around the underlying string. Oracle possibly does something else but it has a real Clob support on protocol and storage level, which ignite doesn't.  In ignite Clob for now is just a thin wrapper around the Strings.
   
   General JDBC application wouldn't create a stream to **read** data from Clob from ResutSet and **change** it at the same time. 
   
   As far as the char[] implementation it would increase the memory usage in the "read" case. It would require the creation of extra char[] array in the ResultSet::getClob.  With current implementation just a string reference is passed and no extra memory is needed (especially if the get*Stream() APIs are used by the application).



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-6141 JDBC: add basic support for BLOB and CLOB types [ignite]

Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #11492:
URL: https://github.com/apache/ignite/pull/11492#discussion_r1743126965


##########
modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/IgniteH2Indexing.java:
##########
@@ -1090,14 +1091,14 @@ private List<? extends FieldsQueryCursor<List<?>>> executeDml(
             if (roEx != null) {
                 throw new IgniteSQLException(
                     "Failed to execute DML statement. Cluster in read-only mode [stmt=" + qryDesc.sql() +
-                    ", params=" + Arrays.deepToString(qryParams.arguments()) + "]",
+                    ", params=" + S.toString(QueryParameters.class, qryParams) + "]",
                     IgniteQueryErrorCode.CLUSTER_READ_ONLY_MODE_ENABLED,
                     e
                 );
             }
 
             throw new IgniteSQLException("Failed to execute DML statement [stmt=" + qryDesc.sql() +
-                ", params=" + Arrays.deepToString(qryParams.arguments()) + "]", e);
+                    ", params=" + S.toString(QueryParameters.class, qryParams) + "]", e);

Review Comment:
   Why this change?



##########
modules/core/src/main/java/org/apache/ignite/internal/jdbc2/JdbcClob.java:
##########
@@ -0,0 +1,292 @@
+/*
+ * 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.jdbc2;
+
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.Reader;
+import java.io.StringReader;
+import java.io.Writer;
+import java.sql.Clob;
+import java.sql.SQLException;
+import java.sql.SQLFeatureNotSupportedException;
+import org.apache.ignite.internal.util.typedef.internal.U;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+/**
+ * CLOB implementation for Ignite JDBC driver.
+ */
+public class JdbcClob implements Clob {
+    /** CLOB's character sequence. */
+    private String chars;
+
+    /**
+     * @param chars CLOB's character sequence.
+     */
+    public JdbcClob(String chars) {
+        this.chars = chars;
+    }
+
+    /** {@inheritDoc} */
+    @Override public long length() throws SQLException {
+        ensureNotClosed();
+
+        return chars.length();
+    }
+
+    /** {@inheritDoc} */
+    @Override public String getSubString(long pos, int len) throws SQLException {
+        ensureNotClosed();
+
+        if (pos < 1 || len < 0 || pos - 1 + len > chars.length())

Review Comment:
   Can we cast `pos` to standard value as a first step to overcome API inconsistency and make code more readable:
   
   ```
   pos = pos - 1;
   
          if (pos < 0 || len < 0 || pos + len > chars.length())
               throw new SQLException("Invalid argument. Position should be greater than 0. Length should not be " +
                   "negative. Position + length should be less than CLOB size [pos=" + (pos+1) + ", length=" + len + ']');
   
           return chars.substring((int)pos, (int)pos + len);
   ```



##########
modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/IgniteH2Indexing.java:
##########
@@ -1090,14 +1091,14 @@ private List<? extends FieldsQueryCursor<List<?>>> executeDml(
             if (roEx != null) {
                 throw new IgniteSQLException(
                     "Failed to execute DML statement. Cluster in read-only mode [stmt=" + qryDesc.sql() +
-                    ", params=" + Arrays.deepToString(qryParams.arguments()) + "]",
+                    ", params=" + S.toString(QueryParameters.class, qryParams) + "]",

Review Comment:
   Why this change?



##########
modules/clients/src/test/java/org/apache/ignite/internal/jdbc2/JdbcClobTest.java:
##########
@@ -0,0 +1,744 @@
+/*
+ * 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.jdbc2;
+
+import java.io.InputStream;
+import java.io.Reader;
+import java.sql.Clob;
+import java.sql.SQLException;
+import java.sql.SQLFeatureNotSupportedException;
+import org.apache.commons.io.IOUtils;
+import org.junit.Assert;
+import org.junit.Test;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+/**
+ * Test for JDBC CLOB.
+ */
+public class JdbcClobTest {
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testLength() throws Exception {
+        JdbcClob clob = new JdbcClob("1234567890");
+
+        assertEquals(10, clob.length());
+
+        clob.free();
+
+        try {

Review Comment:
   Can we use assertThrows here and below?



##########
modules/core/src/main/java/org/apache/ignite/internal/jdbc2/JdbcBlob.java:
##########
@@ -54,7 +54,7 @@ public JdbcBlob(byte[] arr) {
     @Override public byte[] getBytes(long pos, int len) throws SQLException {
         ensureNotClosed();
 
-        if (pos < 1 || arr.length - pos < 0 || len < 0)
+        if (pos < 1 || (arr.length - pos < 0 && arr.length > 0) || len < 0)

Review Comment:
   Can you, please, clarify - why do we need new clause `arr.length > 0`?



##########
modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/QueryParameters.java:
##########
@@ -20,13 +20,16 @@
 
 import java.util.List;
 import org.apache.ignite.internal.processors.query.NestedTxMode;
+import org.apache.ignite.internal.util.tostring.GridToStringInclude;
+import org.apache.ignite.internal.util.typedef.internal.S;
 
 /**
  * Query parameters which vary between requests having the same execution plan. Essentially, these are the arguments
  * of original {@link org.apache.ignite.cache.query.SqlFieldsQuery} which are not part of {@link QueryDescriptor}.
  */
 public class QueryParameters {
     /** Arguments. */
+    @GridToStringInclude(sensitive = true)

Review Comment:
   Is this only field that will be printed out?
   Don't we need other fields?



##########
modules/core/src/main/java/org/apache/ignite/internal/jdbc2/JdbcClob.java:
##########
@@ -0,0 +1,292 @@
+/*
+ * 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.jdbc2;
+
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.Reader;
+import java.io.StringReader;
+import java.io.Writer;
+import java.sql.Clob;
+import java.sql.SQLException;
+import java.sql.SQLFeatureNotSupportedException;
+import org.apache.ignite.internal.util.typedef.internal.U;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+/**
+ * CLOB implementation for Ignite JDBC driver.
+ */
+public class JdbcClob implements Clob {

Review Comment:
   Can we migrate to the `char[]` based implementation?
   It's not clear from the API, but looks like `Reader getCharacterStream()` or `getAsciiStream()` expects to see changes in underlying object if they happens after `getCharacterStream()` invocation.



##########
modules/core/src/main/java/org/apache/ignite/internal/jdbc2/JdbcClob.java:
##########
@@ -0,0 +1,292 @@
+/*
+ * 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.jdbc2;
+
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.Reader;
+import java.io.StringReader;
+import java.io.Writer;
+import java.sql.Clob;
+import java.sql.SQLException;
+import java.sql.SQLFeatureNotSupportedException;
+import org.apache.ignite.internal.util.typedef.internal.U;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+/**
+ * CLOB implementation for Ignite JDBC driver.
+ */
+public class JdbcClob implements Clob {

Review Comment:
   Let's fix all `pos` argument somehow to make value 0 based and consistent with String and any other API.
   For example, make copy of all methods with `pos`, like:
   
   ```
   public String getSubString(int pos, int len) {
       return getSubStringInternal(pos - 1, len);
   }
   
   private getSubstringInternal(int pos, int len) {
       // 0 index based implementation.
   }
   ```



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-6141 JDBC: add basic support for BLOB and CLOB types [ignite]

Posted by "skorotkov (via GitHub)" <gi...@apache.org>.
skorotkov commented on code in PR #11492:
URL: https://github.com/apache/ignite/pull/11492#discussion_r1743381137


##########
modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/IgniteH2Indexing.java:
##########
@@ -1090,14 +1091,14 @@ private List<? extends FieldsQueryCursor<List<?>>> executeDml(
             if (roEx != null) {
                 throw new IgniteSQLException(
                     "Failed to execute DML statement. Cluster in read-only mode [stmt=" + qryDesc.sql() +
-                    ", params=" + Arrays.deepToString(qryParams.arguments()) + "]",
+                    ", params=" + S.toString(QueryParameters.class, qryParams) + "]",

Review Comment:
   If one of the query parameter is huge (say large CLOB/BLOB) usage of _Arrays.deepToString_ does two "bad" things which increases heap memory usage:
   
   - creates full string representation of the large argument
   - once an exception is logged this large string is passed to logging framework 
   
   This can cause unneeded and unexpected OOM error just because of logging. The actual error is not logged in such case. Say I get the following error stack during testing:
   
   ```
   [2024-08-29T22:03:37,692][ERROR][client-connector-#149%jdbc_thin_blob_test.JdbcThinLobTest.test_jdbc_thin_lob.blob_size.1073741824.clob_size.1024.server_heap.12.client_heap.8.ignite_version.dev%] ....
   java.lang.OutOfMemoryError: null
   at java.base/java.lang.AbstractStringBuilder.hugeCapacity(AbstractStringBuilder.java:214) ~[?:?]
   at java.base/java.lang.AbstractStringBuilder.newCapacity(AbstractStringBuilder.java:206) ~[?:?]
   at java.base/java.lang.AbstractStringBuilder.ensureCapacityInternal(AbstractStringBuilder.java:173) ~[?:?]
   at java.base/java.lang.AbstractStringBuilder.append(AbstractStringBuilder.java:775) ~[?:?]
   at java.base/java.lang.StringBuilder.append(StringBuilder.java:252) ~[?:?]
   at java.base/java.util.Arrays.toString(Arrays.java:4988) ~[?:?]
   at java.base/java.util.Arrays.deepToString(Arrays.java:5189) ~[?:?]
   at java.base/java.util.Arrays.deepToString(Arrays.java:5161) ~[?:?]
   at org.apache.ignite.internal.processors.query.h2.IgniteH2Indexing.executeDml(IgniteH2Indexing.java:1100) 
   ```
   
   ***
   
   On the other hand the  _S.toString_ handles huge strings and huge byte arrays with care. It skips most of the data and replaces it with elipsis.   Like  [1, 2, ... and 1073741724 more]  for byte arrays and "aaa ... and 1041353 skipped ... aaa" for strings.
   
   At the similar context SQL arguments are already logged this carefull way.  See for example the JdbcQueryExecuteRequest::toString.   If such a request is added into the log string the huge arguments are stripped. 
   



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-6141 JDBC: add basic support for BLOB and CLOB types [ignite]

Posted by "skorotkov (via GitHub)" <gi...@apache.org>.
skorotkov commented on code in PR #11492:
URL: https://github.com/apache/ignite/pull/11492#discussion_r1743446711


##########
modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/QueryParameters.java:
##########
@@ -20,13 +20,16 @@
 
 import java.util.List;
 import org.apache.ignite.internal.processors.query.NestedTxMode;
+import org.apache.ignite.internal.util.tostring.GridToStringInclude;
+import org.apache.ignite.internal.util.typedef.internal.S;
 
 /**
  * Query parameters which vary between requests having the same execution plan. Essentially, these are the arguments
  * of original {@link org.apache.ignite.cache.query.SqlFieldsQuery} which are not part of {@link QueryDescriptor}.
  */
 public class QueryParameters {
     /** Arguments. */
+    @GridToStringInclude(sensitive = true)

Review Comment:
   Yes, you are right.  It worth adding all parameters as well.  
   
   Looks like In current code the QueryParameters are not logged as a whole.  Only as the  qryParams.arguments().  
   
   But adding other parameters should not break anything and would add more info for errors analisys.
   
   Wil add.



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-6141 JDBC: add basic support for BLOB and CLOB types [ignite]

Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #11492:
URL: https://github.com/apache/ignite/pull/11492#discussion_r1751799646


##########
modules/clients/src/test/java/org/apache/ignite/jdbc/thin/JdbcThinPreparedStatementSelfTest.java:
##########
@@ -854,6 +858,90 @@ public void testArray() throws Exception {
         assert cnt == 1;
     }
 
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testBlob() throws Exception {
+        stmt = conn.prepareStatement(SQL_PART + " where blobVal is not distinct from ?");
+
+        Blob blob = conn.createBlob();
+
+        blob.setBytes(1, new byte[] {1});
+
+        stmt.setBlob(1, blob);
+
+        ResultSet rs = stmt.executeQuery();
+
+        int cnt = 0;
+
+        while (rs.next()) {
+            if (cnt == 0)
+                assert rs.getInt("id") == 1;
+
+            cnt++;
+        }
+
+        assertEquals(1, cnt);
+
+        stmt.setNull(1, BLOB);
+
+        rs = stmt.executeQuery();
+
+        cnt = 0;
+
+        while (rs.next()) {
+            if (cnt == 0)
+                assert rs.getInt("id") == 2;
+
+            cnt++;
+        }
+
+        assert cnt == 1;
+    }
+
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testClob() throws Exception {
+        stmt = conn.prepareStatement(SQL_PART + " where clobVal is not distinct from ?");
+
+        Clob clob = conn.createClob();
+
+        clob.setString(1, "large str");
+
+        stmt.setClob(1, clob);
+
+        ResultSet rs = stmt.executeQuery();
+
+        int cnt = 0;
+
+        while (rs.next()) {
+            if (cnt == 0)
+                assert rs.getInt("id") == 1;
+
+            cnt++;
+        }
+
+        assertEquals(1, cnt);

Review Comment:
   ```suggestion
           assertTrue(rs.next());
           assertEquals(1, rs.getInt("id");
           assertFalse(rs.next());```



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-6141 JDBC: add basic support for BLOB and CLOB types [ignite]

Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #11492:
URL: https://github.com/apache/ignite/pull/11492#discussion_r1751786818


##########
modules/clients/src/test/java/org/apache/ignite/internal/jdbc2/JdbcPreparedStatementSelfTest.java:
##########
@@ -582,7 +586,49 @@ public void testBlob() throws Exception {
 
         assertEquals(1, cnt);
 
-        stmt.setNull(1, BINARY);
+        stmt.setNull(1, BLOB);
+
+        rs = stmt.executeQuery();
+
+        cnt = 0;
+
+        while (rs.next()) {
+            if (cnt == 0)
+                assert rs.getInt("id") == 2;
+
+            cnt++;
+        }
+
+        assert cnt == 1;
+    }
+
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testClob() throws Exception {
+        stmt = conn.prepareStatement("select * from TestObject where clobVal is not distinct from ?");
+
+        Clob clob = conn.createClob();
+
+        clob.setString(1, "large str");
+
+        stmt.setClob(1, clob);
+
+        ResultSet rs = stmt.executeQuery();
+
+        int cnt = 0;
+
+        while (rs.next()) {
+            if (cnt == 0)
+                assert rs.getInt("id") == 1;
+
+            cnt++;
+        }
+
+        assertEquals(1, cnt);

Review Comment:
   ```suggestion
           assertTrue(rs.next());
           assertEquals(1, rs.getInt("id"));
           assertFalse(rs.next());
   ```



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-6141 JDBC: add basic support for BLOB and CLOB types [ignite]

Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #11492:
URL: https://github.com/apache/ignite/pull/11492#discussion_r1751788372


##########
modules/clients/src/test/java/org/apache/ignite/internal/jdbc2/JdbcResultSetSelfTest.java:
##########
@@ -670,6 +675,54 @@ public void testArray() throws Exception {
         assert cnt == 1;
     }
 
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testBlob() throws Exception {
+        ResultSet rs = stmt.executeQuery(SQL);
+
+        int cnt = 0;
+
+        while (rs.next()) {
+            if (cnt == 0) {
+                Blob blob = rs.getBlob("blobVal");
+                Assert.assertArrayEquals(blob.getBytes(1, (int)blob.length()), new byte[] {1});
+
+                blob = rs.getBlob(23);
+                Assert.assertArrayEquals(blob.getBytes(1, (int)blob.length()), new byte[] {1});
+            }
+
+            cnt++;
+        }
+
+        assert cnt == 1;

Review Comment:
   ```suggestion
           assertTrue(rs.next());
           Blob blob = rs.getBlob("blobVal");
           Assert.assertArrayEquals(blob.getBytes(1, (int)blob.length()), new byte[] {1});
   
           blob = rs.getBlob(23);
           Assert.assertArrayEquals(blob.getBytes(1, (int)blob.length()), new byte[] {1});
           assertFalse(rs.next());
   ```



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-6141 JDBC: add basic support for BLOB and CLOB types [ignite]

Posted by "skorotkov (via GitHub)" <gi...@apache.org>.
skorotkov commented on code in PR #11492:
URL: https://github.com/apache/ignite/pull/11492#discussion_r1746996957


##########
modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/QueryParameters.java:
##########
@@ -20,13 +20,16 @@
 
 import java.util.List;
 import org.apache.ignite.internal.processors.query.NestedTxMode;
+import org.apache.ignite.internal.util.tostring.GridToStringInclude;
+import org.apache.ignite.internal.util.typedef.internal.S;
 
 /**
  * Query parameters which vary between requests having the same execution plan. Essentially, these are the arguments
  * of original {@link org.apache.ignite.cache.query.SqlFieldsQuery} which are not part of {@link QueryDescriptor}.
  */
 public class QueryParameters {
     /** Arguments. */
+    @GridToStringInclude(sensitive = true)

Review Comment:
   done



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] IGNITE-6141 JDBC: add basic support for BLOB and CLOB types [ignite]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #11492:
URL: https://github.com/apache/ignite/pull/11492#issuecomment-2340484313

   ## [![Quality Gate Failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-failed-20px.png 'Quality Gate Failed')](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=11492) **Quality Gate failed**  
   Failed conditions  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/failed-16px.png '') [2 New Code Smells](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=11492) (required ≤ 1)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=11492)
   
   ##   
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/light_bulb-16px.png '') Catch issues before they fail your Quality Gate with our IDE extension ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/sonarlint-16px.png '') [SonarLint](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=pull-request)
   
   


-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org