-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Enhancement]Add session variable enable_compare_for_null to support compare null … #51201
base: main
Are you sure you want to change the base?
[Enhancement]Add session variable enable_compare_for_null to support compare null … #51201
Conversation
public void setEnableCompareForNull(boolean enableCompareForNull) { | ||
this.enableCompareForNull = enableCompareForNull; | ||
} | ||
|
||
public long getSqlMode() { | ||
return sqlMode; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The most risky bug in this code is:
Serialization inconsistency due to missing enableCompareForNull
field.
You can modify the code like this:
@@ -173,6 +173,7 @@ public class SessionVariable implements Serializable, Writable, Cloneable {
public static final String QUERY_CACHE_TYPE = "query_cache_type";
public static final String INTERACTIVE_TIMEOUT = "interactive_timeout";
public static final String WAIT_TIMEOUT = "wait_timeout";
+ public static final String ENABLE_COMPARE_FOR_NULL = "enable_compare_for_null";
public static final String CATALOG = "catalog";
public static final String NET_WRITE_TIMEOUT = "net_write_timeout";
@@ -1005,6 +1006,9 @@ public static MaterializedViewRewriteMode parse(String str) {
@VariableMgr.VarAttr(name = WAIT_TIMEOUT)
private int waitTimeout = 28800;
+ @VariableMgr.VarAttr(name = ENABLE_COMPARE_FOR_NULL)
+ private boolean enableCompareForNull = false;
+
// The number of seconds to wait for a block to be written to a connection before aborting the write
@VariableMgr.VarAttr(name = NET_WRITE_TIMEOUT)
private int netWriteTimeout = 60;
@@ -2610,6 +2614,14 @@ public int getWaitTimeoutS() {
return waitTimeout;
}
+ public boolean isEnableCompareForNull() {
+ return enableCompareForNull;
+ }
+
+ public void setEnableCompareForNull(boolean enableCompareForNull) {
+ this.enableCompareForNull = enableCompareForNull;
+ }
+
public long getSqlMode() {
return sqlMode;
}
+ /**
+ * Ensure that enableCompareForNull is serialized correctly.
+ */
+ private void writeObject(ObjectOutputStream out) throws IOException {
+ out.defaultWriteObject();
+ out.writeBoolean(enableCompareForNull);
+ }
+
+ /**
+ * Ensure that enableCompareForNull is deserialized correctly.
+ */
+ private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException {
+ in.defaultReadObject();
+ enableCompareForNull = in.readBoolean();
+ }
00e2bb5
to
2ec28e2
Compare
@Mergifyio rebase main |
…when use = Signed-off-by: evenhuang <986025158@qq.com>
✅ Branch has been successfully rebased |
2ec28e2
to
4342b98
Compare
Quality Gate passedIssues Measures |
[Java-Extensions Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
[BE Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
[FE Incremental Coverage Report]✅ pass : 2 / 2 (100.00%) file detail
|
I think I should let the user modify the SQL rather than support this unreasonable behavior. |
We actually hope that users can modify SQL, but due to some historical reasons, a large number of this type of SQL need to be modified. Users think that the workload is huge and there are certain risks in modification, so they hope that we support this feature. Moreover, for SR, supporting this parameter can meet more user scenarios, which is actually beneficial. In addition, it does not affect the original query by default. Users who need this feature can only enable it through parameters. If some users are unwilling to use SR because of this situation, I think this is actually a lose-lose result, so I think it is more reasonable to support this behavior through parameters. |
…when use =
need use = to compare null = null and want to it return true
Why I'm doing:
Some services migrated from other systems want to use = for null comparison, but do not want to modify the SQL
What I'm doing:
add the session variable enable_compare_for_null to support compare null with =
Fixes #issue
What type of PR is this:
Does this PR entail a change in behavior?
If yes, please specify the type of change:
Checklist:
Bugfix cherry-pick branch check: