Skip to content
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

fix: the results of tikv and tiflash are different (#5839) #5874

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion dbms/src/Functions/FunctionsLogical.h
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,23 @@ struct AssociativeOperationImpl
{
if (Op::isSaturable())
{
UInt8 a = vec[i];
// cast a: UInt8 -> bool -> UInt8 is a trick
// TiFlash converts columns with non-UInt8 type to UInt8 type and sets value to 0 or 1
// which correspond to false or true. However, for columns with UInt8 type,
// no more convertion will be executed on them and the values stored
// in them are 'origin' which means that they won't be converted to 0 or 1.
// For example:
// Input column with non-UInt8 type:
// column_values = {-2, 0, 2}
// then, they will be converted to:
// vec = {1, 0, 1} (here vec stores converted values)
//
// Input column with UInt8 type:
// column_values = {1, 0, 2}
// then, the vec will be:
// vec = {1, 0, 2} (error, we only want 0 or 1)
// See issue: https://github.com/pingcap/tidb/issues/37258
bool a = static_cast<bool>(vec[i]);
return Op::isSaturatedValue(a) ? a : continuation.apply(i);
}
else
Expand Down
7 changes: 7 additions & 0 deletions dbms/src/Functions/tests/gtest_logical.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,13 @@ try
func_name,
createColumn<Nullable<UInt8>>({0, 1, 0, 1, {}, 0}),
createColumn<Nullable<UInt8>>({0, 1, 1, 0, 1, {}})));
// issue 5849
ASSERT_COLUMN_EQ(
createColumn<UInt8>({0, 1, 1, 1}),
executeFunction(
func_name,
createColumn<UInt8>({0, 123, 0, 41}),
createColumn<Int64>({0, 11, 221, 0})));
// column, const
ASSERT_COLUMN_EQ(
createColumn<Nullable<UInt8>>({1, 1}),
Expand Down
66 changes: 33 additions & 33 deletions format-diff.py
Original file line number Diff line number Diff line change
@@ -1,44 +1,48 @@
#!/usr/bin/python3
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import argparse
import os
import subprocess
from os import path

import json


def run_cmd(cmd, show_cmd=False):
res = os.popen(cmd).readlines()
if show_cmd:
print("RUN CMD: {}".format(cmd))
return res


def main():
default_suffix = ['.cpp', '.h', '.cc', '.hpp']
parser = argparse.ArgumentParser(description='TiFlash Code Format',
formatter_class=argparse.ArgumentDefaultsHelpFormatter)
parser.add_argument('--repo_path', help='path of tics repository',
parser.add_argument('--repo_path', help='path of tiflash repository',
default=os.path.dirname(os.path.abspath(__file__)))
parser.add_argument('--suffix',
help='suffix of files to format, split by space', default=' '.join(default_suffix))
parser.add_argument('--ignore_suffix', help='ignore files with suffix, split by space')
parser.add_argument('--diff_from', help='commit hash/branch to check git diff', default='HEAD')
parser.add_argument('--check_formatted', help='exit -1 if NOT formatted', action='store_true')
parser.add_argument('--dump_diff_files_to', help='dump diff file names to specific path', default=None)

parser.add_argument('--ignore_suffix',
help='ignore files with suffix, split by space')
parser.add_argument(
'--diff_from', help='commit hash/branch to check git diff', default='HEAD')
parser.add_argument('--check_formatted',
help='exit -1 if NOT formatted', action='store_true')
parser.add_argument('--dump_diff_files_to',
help='dump diff file names to specific path', default=None)
args = parser.parse_args()
default_suffix = args.suffix.strip().split(' ') if args.suffix else []
ignore_suffix = args.ignore_suffix.strip().split(' ') if args.ignore_suffix else []
tics_repo_path = args.repo_path
if not os.path.isabs(tics_repo_path):
ignore_suffix = args.ignore_suffix.strip().split(
' ') if args.ignore_suffix else []
tiflash_repo_path = args.repo_path
if not os.path.isabs(tiflash_repo_path):
raise Exception("path of repo should be absolute")
assert tics_repo_path[-1] != '/'

os.chdir(tics_repo_path)
assert tiflash_repo_path[-1] != '/'
os.chdir(tiflash_repo_path)
files_to_check = run_cmd('git diff HEAD --name-only') if args.diff_from == 'HEAD' else run_cmd(
'git diff {} --name-only'.format(args.diff_from))
files_to_check = [os.path.join(tics_repo_path, s.strip()) for s in files_to_check]
files_to_check = [os.path.join(tiflash_repo_path, s.strip())
for s in files_to_check]
files_to_format = []
for f in files_to_check:
if not any([f.endswith(e) for e in default_suffix]):
Expand All @@ -52,19 +56,20 @@ def main():
print('file {} can not be formatted'.format(file_path))
continue
files_to_format.append(file_path)

if args.dump_diff_files_to:
da = [e[len(tics_repo_path):] for e in files_to_format]
json.dump({'files': da, 'repo': tics_repo_path}, open(args.dump_diff_files_to, 'w'))
print('dump {} modified files info to {}'.format(len(da), args.dump_diff_files_to))

da = [e[len(tiflash_repo_path):] for e in files_to_format]
json.dump({'files': da, 'repo': tiflash_repo_path},
open(args.dump_diff_files_to, 'w'))
print('dump {} modified files info to {}'.format(
len(da), args.dump_diff_files_to))
if files_to_format:
print('Files to format:\n {}'.format('\n '.join(files_to_format)))
for file in files_to_format:
cmd = 'clang-format -i {}'.format(file)
if subprocess.Popen(cmd, shell=True, cwd=tiflash_repo_path).wait():
exit(-1)

if args.check_formatted:
cmd = 'clang-format -i {}'.format(' '.join(files_to_format))
if subprocess.Popen(cmd, shell=True, cwd=tics_repo_path).wait():
exit(-1)
diff_res = run_cmd('git diff --name-only')
files_not_in_contrib = [f for f in diff_res if not f.startswith('contrib')]
files_contrib = [f for f in diff_res if f.startswith('contrib')]
Expand All @@ -82,13 +87,8 @@ def main():
else:
print("Format check passed")
else:
cmd = 'clang-format -i {}'.format(' '.join(files_to_format))
if subprocess.Popen(cmd, shell=True, cwd=tics_repo_path).wait():
exit(-1)
print("Finish code format")
else:
print('No file to format')


if __name__ == '__main__':
main()
main()
17 changes: 17 additions & 0 deletions tests/fullstack-test/expr/logical_op.test
Original file line number Diff line number Diff line change
@@ -1,19 +1,24 @@
mysql> drop table if exists test.t1;
mysql> drop table if exists test.t2;
mysql> drop table if exists test.t3;
mysql> drop table if exists test.t4;
mysql> create table test.t1(a char(20),b double);
mysql> create table test.t2(a char(20));
mysql> create table test.t3(a int);
mysql> create table test.t4(a tinyint(45) unsigned NOT NULL, b bigint(20) NOT NULL);
mysql> insert into test.t1 values(1,null),('j',0),(1,12.991),(0,0),(0,0),('they',1.009),('can',-99),(0,12.991),(1,-9.183),(null,1);
mysql> insert into test.t2 values(0),(0),(0),(0),(0),(0),(1),('with'),('see'),(null);
mysql> insert into test.t3 values(0),(1);
mysql> insert into test.t4 values(65, 1),(66, 2), (67, 3), (0, 0);
mysql> alter table test.t1 set tiflash replica 1;
mysql> alter table test.t2 set tiflash replica 1;
mysql> alter table test.t3 set tiflash replica 1;
mysql> alter table test.t4 set tiflash replica 1;

func> wait_table test t1
func> wait_table test t2
func> wait_table test t3
func> wait_table test t4

mysql> set session tidb_isolation_read_engines='tiflash'; select count(*) from test.t1 where (b between null and 100) is null;
+----------+
Expand Down Expand Up @@ -67,6 +72,18 @@ mysql> set tidb_enforce_mpp=1; set tidb_isolation_read_engines='tiflash'; select
mysql> set tidb_enforce_mpp=1; set tidb_isolation_read_engines='tiflash'; select count(*) from test.t3 group by a having ifnull(null,count(*)) and min(null);
# empty

# issue 5849
mysql> set tidb_enforce_mpp=1; set tidb_isolation_read_engines='tiflash'; select a or b from test.t4;
+--------+
| a or b |
+--------+
| 1 |
| 1 |
| 1 |
| 0 |
+--------+

#mysql> drop table if exists test.t1;
#mysql> drop table if exists test.t2;
#mysql> drop table if exists test.t3;
#mysql> drop table if exists test.t4;