You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2020/02/04 08:51:01 UTC

[GitHub] [incubator-doris] lingbin opened a new pull request #2833: Improve initialization flow of Schema

lingbin opened a new pull request #2833: Improve initialization flow of Schema
URL: https://github.com/apache/incubator-doris/pull/2833
 
 
   When constructing `Schema` objects, two similar `init` functions
   need to be called, and the call order is implicitly required, which
   is easy to be misused. At the same time, some of the existing comments
   are missing or out of date, which will cause some misleading.
   
   This patch unifies the initialization logic of `Schema`.
   
   No functional changes in this patch.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] lingbin commented on a change in pull request #2833: Improve initialization flow of Schema

Posted by GitBox <gi...@apache.org>.
lingbin commented on a change in pull request #2833: Improve initialization flow of Schema
URL: https://github.com/apache/incubator-doris/pull/2833#discussion_r374629084
 
 

 ##########
 File path: be/src/olap/schema.h
 ##########
 @@ -120,39 +119,36 @@ class Schema {
     bool is_null(const char* row, int index) const {
         return *reinterpret_cast<const bool*>(row + _col_offsets[index]);
     }
-
     void set_is_null(void* row, uint32_t cid, bool is_null) const {
         *reinterpret_cast<bool*>((char*)row + _col_offsets[cid]) = is_null;
     }
 
-    size_t schema_size() const {
-        return _schema_size;
-    }
-
     size_t num_columns() const { return _cols.size(); }
     size_t num_column_ids() const { return _col_ids.size(); }
     const std::vector<ColumnId>& column_ids() const { return _col_ids; }
+
 private:
-    // all valid ColumnIds in this schema
+    void _init(const std::vector<TabletColumn>& cols,
+               const std::vector<ColumnId>& col_ids,
+               size_t num_key_columns);
+    void _init(const std::vector<const Field*>& cols,
+               const std::vector<ColumnId>& col_ids,
+               size_t num_key_columns);
+
+    void _copy_from(const Schema& other);
+
+    // NOTE: The ColumnId here represents the sequential index number (starting from 0) of
+    // a column in current row, not the unique id-identifier of each column
 
 Review comment:
   It seems that the type alias `ColumnId` really means `ColumnIdx` in our codeļ¼Œso It was left there unmodified.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman commented on a change in pull request #2833: Improve initialization flow of Schema

Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #2833: Improve initialization flow of Schema
URL: https://github.com/apache/incubator-doris/pull/2833#discussion_r374588661
 
 

 ##########
 File path: be/src/olap/schema.h
 ##########
 @@ -120,39 +119,36 @@ class Schema {
     bool is_null(const char* row, int index) const {
         return *reinterpret_cast<const bool*>(row + _col_offsets[index]);
     }
-
     void set_is_null(void* row, uint32_t cid, bool is_null) const {
         *reinterpret_cast<bool*>((char*)row + _col_offsets[cid]) = is_null;
     }
 
-    size_t schema_size() const {
-        return _schema_size;
-    }
-
     size_t num_columns() const { return _cols.size(); }
     size_t num_column_ids() const { return _col_ids.size(); }
     const std::vector<ColumnId>& column_ids() const { return _col_ids; }
+
 private:
-    // all valid ColumnIds in this schema
+    void _init(const std::vector<TabletColumn>& cols,
+               const std::vector<ColumnId>& col_ids,
+               size_t num_key_columns);
+    void _init(const std::vector<const Field*>& cols,
+               const std::vector<ColumnId>& col_ids,
+               size_t num_key_columns);
+
+    void _copy_from(const Schema& other);
+
+    // NOTE: The ColumnId here represents the sequential index number (starting from 0) of
+    // a column in current row, not the unique id-identifier of each column
 
 Review comment:
   if `_col_ids` is what you mean in comment, why not just using `int32` instead of `ColumnId` ?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman merged pull request #2833: Improve initialization flow of Schema

Posted by GitBox <gi...@apache.org>.
morningman merged pull request #2833: Improve initialization flow of Schema
URL: https://github.com/apache/incubator-doris/pull/2833
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org