xiaokang commented on code in PR #26749:
URL: https://github.com/apache/doris/pull/26749#discussion_r1391877925
##########
be/src/common/config.cpp:
##########
@@ -1108,6 +1109,11 @@ DEFINE_Int32(ingest_binlog_work_pool_size, "-1");
// Download binlog rate limit, unit is KB/s, 0 means no limit
DEFINE_Int32(download_binlog_rate_limit_kbs, "0");
+// Buffer block size for flushing a single block
+// This config used for merging multiple blocks into a single block
+// and flush them once total, default 400M
+DEFINE_mInt32(flushing_block_buffer_size_bytes, "419430400");
Review Comment:
When is this config used? The default value 400M is large.
##########
gensrc/proto/internal_service.proto:
##########
@@ -589,6 +589,7 @@ message PTabletWriteSlaveRequest {
message IndexSize {
required int64 indexId = 1;
required int64 size = 2;
+ required string suffix_path = 3;
Review Comment:
What's the meaning of suffix_path and why put it in IndexSize?
##########
be/src/common/config.cpp:
##########
@@ -946,6 +946,7 @@
DEFINE_Bool(enable_index_apply_preds_except_leafnode_of_andnode, "true");
DEFINE_mBool(enable_flatten_nested_for_variant, "false");
DEFINE_mDouble(ratio_of_defaults_as_sparse_column, "0.95");
+DEFINE_mInt64(threshold_rows_to_estimate_sparse_column, "1000");
Review Comment:
use variant_ prefix for all variant config
##########
fe/fe-core/src/main/cup/sql_parser.cup:
##########
@@ -7049,6 +7066,16 @@ column_ref ::=
{: RESULT = new SlotRef(new TableName(null, db, tbl), col); :}
| ident:ctl DOT ident:db DOT ident:tbl DOT ident:col
{: RESULT = new SlotRef(new TableName(ctl, db, tbl), col); :}
+
+ | ident:pcol COLON sub_column_lables:lables
Review Comment:
As discussed, use DOT instead of COLON.
##########
fe/fe-core/src/main/cup/sql_parser.cup:
##########
@@ -764,6 +765,7 @@ nonterminal String sequence_col_clause;
nonterminal Predicate predicate, between_predicate, comparison_predicate,
compound_predicate, in_predicate, like_predicate, exists_predicate,
match_predicate;
nonterminal ArrayList<Expr> opt_partition_by_clause;
+nonterminal ArrayList<String> sub_column_lables;
Review Comment:
suggest name : sub_column_names
##########
fe/fe-core/src/main/cup/sql_parser.cup:
##########
@@ -440,6 +440,7 @@ terminal String
KW_JOIN,
KW_JSON,
KW_JSONB,
+ KW_VARIANT,
Review Comment:
Is there no keyword KW_VARIANT in the load pr?
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/SlotDescriptor.java:
##########
@@ -64,7 +67,6 @@ public class SlotDescriptor {
private ColumnStats stats; // only set if 'column' isn't set
private boolean isAgg;
- private boolean isMultiRef;
Review Comment:
What's the meaning of isMultiRef and why delete it?
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/SlotRef.java:
##########
@@ -368,6 +392,13 @@ public boolean notCheckDescIdEquals(Object obj) {
if (col != null && !col.equalsIgnoreCase(other.col)) {
return false;
}
+ if ((subColLables == null) != (other.subColLables == null)) {
+ return false;
+ }
+ if (subColLables != null
+ && String.join(".", subColLables).equals(String.join(".",
other.subColLables))) {
Review Comment:
just check subColLables.equals(other.subColLables)
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/Analyzer.java:
##########
@@ -1004,11 +1023,49 @@ public SlotDescriptor registerColumnRef(TableName
tblName, String colName) throw
newTblName == null ?
d.getTable().getName() : newTblName.toString());
}
+ LOG.debug("register column ref table {}, colName {}, col {}", tblName,
colName, col.toSql());
+ if (col.getType().isVariantType() || (subColNames != null &&
!subColNames.isEmpty())) {
+ if (!col.getType().isVariantType()) {
+
ErrorReport.reportAnalysisException(ErrorCode.ERR_ILLEGAL_COLUMN_REFERENCE_ERROR,
+ Joiner.on(".").join(tblName.getTbl(), colName));
+ }
+ if (subColNames == null) {
+ // Root
+ subColNames = new ArrayList<String>();
+ }
+ String key = d.getAlias() + "." + col.getName();
+ if (subColumnSlotRefMap.get(key) == null) {
+ subColumnSlotRefMap.put(key, Maps.newTreeMap(
+ new Comparator<List<String>>() {
+ public int compare(List<String> lst1, List<String>
lst2) {
+ String str1 = String.join(".", lst1);
+ String str2 = String.join(".", lst2);
+ return str1.compareTo(str2);
+ }
+ }));
+ }
+ SlotDescriptor result =
subColumnSlotRefMap.get(key).get(subColNames);
+ if (result != null) {
+ // avoid duplicate slots
+ return result;
+ }
+ result = globalState.descTbl.addSlotDescriptor(d);
Review Comment:
Do you mean add SlotDescriptor for each subcolumn? Is it conflict with
original behevior that one SlotDescriptor for each column.
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/Analyzer.java:
##########
@@ -1004,11 +1023,49 @@ public SlotDescriptor registerColumnRef(TableName
tblName, String colName) throw
newTblName == null ?
d.getTable().getName() : newTblName.toString());
}
+ LOG.debug("register column ref table {}, colName {}, col {}", tblName,
colName, col.toSql());
+ if (col.getType().isVariantType() || (subColNames != null &&
!subColNames.isEmpty())) {
+ if (!col.getType().isVariantType()) {
+
ErrorReport.reportAnalysisException(ErrorCode.ERR_ILLEGAL_COLUMN_REFERENCE_ERROR,
+ Joiner.on(".").join(tblName.getTbl(), colName));
+ }
+ if (subColNames == null) {
+ // Root
+ subColNames = new ArrayList<String>();
+ }
+ String key = d.getAlias() + "." + col.getName();
+ if (subColumnSlotRefMap.get(key) == null) {
+ subColumnSlotRefMap.put(key, Maps.newTreeMap(
+ new Comparator<List<String>>() {
+ public int compare(List<String> lst1, List<String>
lst2) {
+ String str1 = String.join(".", lst1);
+ String str2 = String.join(".", lst2);
+ return str1.compareTo(str2);
+ }
+ }));
+ }
+ SlotDescriptor result =
subColumnSlotRefMap.get(key).get(subColNames);
+ if (result != null) {
+ // avoid duplicate slots
+ return result;
+ }
+ result = globalState.descTbl.addSlotDescriptor(d);
+ LOG.debug("register slot descriptor {}", result);
+ result.setSubColLables(subColNames);
+ result.setColumn(col);
+ if (!subColNames.isEmpty()) {
+ result.setMaterializedColumnName(col.getName() + "." +
String.join(".", subColNames));
+ }
+ result.setIsMaterialized(true);
+ result.setIsNullable(col.isAllowNull());
+ subColumnSlotRefMap.get(key).put(subColNames, result);
+ return result;
+ }
+
// Make column name case insensitive
String key = d.getAlias() + "." + col.getName();
SlotDescriptor result = slotRefMap.get(key);
if (result != null) {
- result.setMultiRef(true);
return result;
}
result = globalState.descTbl.addSlotDescriptor(d);
Review Comment:
So, there is not SlotDescriptor for the top variant column self.
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/SelectListItem.java:
##########
@@ -149,6 +151,20 @@ public String toColumnLabel(int position) {
return "__" + expr.getExprName() + "_" + position;
}
+ public List<String> toSubColumnLabels() {
+ Preconditions.checkState(!isStar());
+ // if (alias != null) {
Review Comment:
delete useless code
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/Analyzer.java:
##########
@@ -759,6 +765,18 @@ public TupleDescriptor registerOlapTable(Table table,
TableName tableName, List<
slot.setIsNullable(col.isAllowNull());
String key = tableRef.aliases[0] + "." + col.getName();
slotRefMap.put(key, slot);
+
+ if (col.getType().isVariantType()) {
+ LOG.debug("add subColumnSlotRefMap, key {}, column {}", key,
col.toThrift());
+ subColumnSlotRefMap.put(key, Maps.newTreeMap(
Review Comment:
Is this duplicated code necessary since it can be handled by
registerColumnRef?
##########
gensrc/proto/olap_file.proto:
##########
@@ -223,6 +223,7 @@ message TabletIndexPB {
optional IndexType index_type = 3;
repeated int32 col_unique_id = 4;
map<string, string> properties = 5;
+ optional string index_suffix_name = 6;
Review Comment:
?
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/SlotRef.java:
##########
@@ -57,6 +57,7 @@ public class SlotRef extends Expr {
private String col;
// Used in toSql
private String label;
+ private List<String> subColLables;
Review Comment:
suggest name: subColNames or subCols
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/SlotRef.java:
##########
@@ -315,6 +327,11 @@ public String getExprName() {
return this.exprName.get();
}
+ public List<String> toSubColumnLabel() {
+ // return tblName == null ? col : tblName.getTbl() + "." + col;
Review Comment:
delete useless code
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/SlotDescriptor.java:
##########
@@ -44,6 +44,9 @@ public class SlotDescriptor {
// for SlotRef.toSql() in the absence of a path
private String label;
+ // for variant column's sub column lables
+ private List<String> subColLabels;
+ private String materializedColumnnName;
Review Comment:
What's the meaning of materializedColumnnName? Add comment for it.
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/Analyzer.java:
##########
@@ -1004,11 +1023,49 @@ public SlotDescriptor registerColumnRef(TableName
tblName, String colName) throw
newTblName == null ?
d.getTable().getName() : newTblName.toString());
}
+ LOG.debug("register column ref table {}, colName {}, col {}", tblName,
colName, col.toSql());
+ if (col.getType().isVariantType() || (subColNames != null &&
!subColNames.isEmpty())) {
+ if (!col.getType().isVariantType()) {
+
ErrorReport.reportAnalysisException(ErrorCode.ERR_ILLEGAL_COLUMN_REFERENCE_ERROR,
+ Joiner.on(".").join(tblName.getTbl(), colName));
+ }
+ if (subColNames == null) {
+ // Root
+ subColNames = new ArrayList<String>();
+ }
+ String key = d.getAlias() + "." + col.getName();
+ if (subColumnSlotRefMap.get(key) == null) {
+ subColumnSlotRefMap.put(key, Maps.newTreeMap(
+ new Comparator<List<String>>() {
+ public int compare(List<String> lst1, List<String>
lst2) {
+ String str1 = String.join(".", lst1);
+ String str2 = String.join(".", lst2);
+ return str1.compareTo(str2);
Review Comment:
as before
##########
gensrc/proto/segment_v2.proto:
##########
@@ -183,7 +183,6 @@ message ColumnMetaPB {
// required by array/struct/map reader to create child reader.
optional uint64 num_rows = 11;
repeated string children_column_names = 12;
-
Review Comment:
useless change
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/Analyzer.java:
##########
@@ -759,6 +765,18 @@ public TupleDescriptor registerOlapTable(Table table,
TableName tableName, List<
slot.setIsNullable(col.isAllowNull());
String key = tableRef.aliases[0] + "." + col.getName();
slotRefMap.put(key, slot);
+
+ if (col.getType().isVariantType()) {
+ LOG.debug("add subColumnSlotRefMap, key {}, column {}", key,
col.toThrift());
+ subColumnSlotRefMap.put(key, Maps.newTreeMap(
+ new Comparator<List<String>>() {
+ public int compare(List<String> lst1, List<String>
lst2) {
+ String str1 = String.join(".", lst1);
+ String str2 = String.join(".", lst2);
+ return str1.compareTo(str2);
Review Comment:
lst1.compareTo(lst2) may be ok
--
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: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
