morningman commented on code in PR #15250:
URL: https://github.com/apache/doris/pull/15250#discussion_r1057079041
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/CreateTableStmt.java:
##########
@@ -94,6 +96,32 @@ public class CreateTableStmt extends DdlStmt {
engineNames.add("jdbc");
}
+ // if auto bucket auto bucket enable, rewrite distribution bucket num &&
+ // set properties[PropertyAnalyzer.PROPERTIES_AUTO_BUCKET] = "true"
+ private static Map<String, String>
maybeRewriteByAutoBucket(DistributionDesc distributionDesc,
+ Map<String, String> properties) throws AnalysisException {
+ if (distributionDesc == null || !distributionDesc.isAutoBucket()) {
+ return properties;
+ }
+
+ // auto bucket is enable
+ Map<String, String> newProperties = properties;
+ if (newProperties == null) {
+ newProperties = new HashMap<String, String>();
+ }
+ newProperties.put(PropertyAnalyzer.PROPERTIES_AUTO_BUCKET, "true");
+
+ if
(!newProperties.containsKey(PropertyAnalyzer.PROPERTIES_ESTIMATE_PARTITION_SIZE))
{
+ distributionDesc.setBuckets(11);
Review Comment:
Define this `11` in `FeConstants.java`
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/DistributionInfo.java:
##########
@@ -40,12 +40,28 @@ public enum DistributionInfoType {
@SerializedName(value = "type")
protected DistributionInfoType type;
+ @SerializedName(value = "bucketNum")
+ protected int bucketNum;
Review Comment:
Same suggestions as `DistributionDesc`
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/OlapTable.java:
##########
@@ -1278,6 +1277,9 @@ public void readFields(DataInput in) throws IOException {
if (in.readBoolean()) {
tableProperty = TableProperty.read(in);
}
+ if (isAutoBucket()) {
+ defaultDistributionInfo.markAutoBucket();
Review Comment:
You have read the `defaultDistributionInfo` in line 1243, so no need to mark
auto bucket again.
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/DistributionDesc.java:
##########
@@ -52,27 +70,10 @@ public DistributionInfo toDistributionInfo(List<Column>
columns) throws DdlExcep
throw new NotImplementedException();
}
- public static DistributionDesc read(DataInput in) throws IOException {
- DistributionInfoType type =
DistributionInfoType.valueOf(Text.readString(in));
- if (type == DistributionInfoType.HASH) {
- DistributionDesc desc = new HashDistributionDesc();
- desc.readFields(in);
- return desc;
- } else if (type == DistributionInfoType.RANDOM) {
- DistributionDesc desc = new RandomDistributionDesc();
- desc.readFields(in);
- return desc;
- } else {
- throw new IOException("Unknown distribution type: " + type);
- }
- }
-
@Override
public void write(DataOutput out) throws IOException {
Text.writeString(out, type.name());
- }
-
- public void readFields(DataInput in) throws IOException {
- throw new NotImplementedException();
+ out.writeInt(numBucket);
Review Comment:
You can not change `write` method like this, this will cause metadata
incompatible.
I suggest to not modify the `write` and `read` method of `DistributionDesc`
and its derived classes.
And also leave the `numBucket` and `autoBucket` in derived classes.
And you can add `getBuckets()` and `setBuckets()` method and override them
in derived classes.
This maybe duplicated, but will not broke the metadata compatibility.
--
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
