You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "isapego (via GitHub)" <gi...@apache.org> on 2023/09/05 14:17:34 UTC

[GitHub] [ignite-3] isapego opened a new pull request, #2551: IGNITE-19214 Table metadata fetching for ODBC

isapego opened a new pull request, #2551:
URL: https://github.com/apache/ignite-3/pull/2551

   - Implemented table meta fetching, re-used `JDBC_TABLE_META` message. Need to refactor in future.
   - Implemented fetching on ODBC side;
   - Un-commented related tests.


-- 
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


[GitHub] [ignite-3] isapego merged pull request #2551: IGNITE-19214 Table metadata fetching for ODBC

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


-- 
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


[GitHub] [ignite-3] isapego commented on a diff in pull request #2551: IGNITE-19214 Table metadata fetching for ODBC

Posted by "isapego (via GitHub)" <gi...@apache.org>.
isapego commented on code in PR #2551:
URL: https://github.com/apache/ignite-3/pull/2551#discussion_r1316109585


##########
modules/platforms/cpp/ignite/odbc/meta/table_meta.cpp:
##########
@@ -20,22 +20,32 @@
 namespace ignite {
 
 void table_meta::read(protocol::reader &reader) {
-    catalog_name = reader.read_string();
+    auto has_data = reader.read_bool();
+    assert(has_data);

Review Comment:
   Well, yeah, there is no a standard way to do that. We should probably implement our version. Filed a ticket: https://issues.apache.org/jira/browse/IGNITE-20352



-- 
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


[GitHub] [ignite-3] isapego commented on a diff in pull request #2551: IGNITE-19214 Table metadata fetching for ODBC

Posted by "isapego (via GitHub)" <gi...@apache.org>.
isapego commented on code in PR #2551:
URL: https://github.com/apache/ignite-3/pull/2551#discussion_r1316112008


##########
modules/platforms/cpp/ignite/odbc/sql_statement.cpp:
##########
@@ -560,7 +561,7 @@ sql_result sql_statement::internal_execute_get_columns_meta_query(
     if (m_current_query)
         m_current_query->close();
 
-    // TODO: IGNITE-19214 Implement table column metadata fetching
+    // TODO: IGNITE-20346 Implement table column metadata fetching

Review Comment:
   Well, yeah, they have different titles though. First one is about table metadata, and the second one is about columns metadata. They are different queries with different functions used to access them.



-- 
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


[GitHub] [ignite-3] ptupitsyn commented on a diff in pull request #2551: IGNITE-19214 Table metadata fetching for ODBC

Posted by "ptupitsyn (via GitHub)" <gi...@apache.org>.
ptupitsyn commented on code in PR #2551:
URL: https://github.com/apache/ignite-3/pull/2551#discussion_r1316046288


##########
modules/platforms/cpp/ignite/odbc/sql_statement.cpp:
##########
@@ -560,7 +561,7 @@ sql_result sql_statement::internal_execute_get_columns_meta_query(
     if (m_current_query)
         m_current_query->close();
 
-    // TODO: IGNITE-19214 Implement table column metadata fetching
+    // TODO: IGNITE-20346 Implement table column metadata fetching

Review Comment:
   [IGNITE-20346](https://issues.apache.org/jira/browse/IGNITE-20346) and [IGNITE-19214](https://issues.apache.org/jira/browse/IGNITE-19214) have exactly the same description. What's the difference?



##########
modules/platforms/cpp/ignite/odbc/meta/table_meta.cpp:
##########
@@ -20,22 +20,32 @@
 namespace ignite {
 
 void table_meta::read(protocol::reader &reader) {
-    catalog_name = reader.read_string();
+    auto has_data = reader.read_bool();
+    assert(has_data);

Review Comment:
   Is it possible to add a message to an `assert`, to understand the problem easier if it ever fires?
   
   The article at https://en.cppreference.com/w/cpp/error/assert says `There is no standardized interface to add an additional message` but provides a couple workarounds.



-- 
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