Skip to content

Commit 89bd042

Browse files
xiangjinwumergify[bot]
authored andcommitted
fix(common): order null as largest by default to align with PostgreSQL (risingwavelabs#4263)
* encode and decode null as largest in memcomparable encoding * fix unit test except lookup join ``` NEXTEST_EXPERIMENTAL_FILTER_EXPR=1 ./risedev test -E 'not test(~lookup_join)' ``` * update lookup join unit test * update e2e * update batch ordering of null * update e2e Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
1 parent 47ece54 commit 89bd042

File tree

6 files changed

+35
-36
lines changed

6 files changed

+35
-36
lines changed

e2e_test/batch/basic/order_by.slt.part

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,17 +91,17 @@ insert into t values (null, 7, null);
9191
query III
9292
select * from t order by v1 limit 2;
9393
----
94-
NULL 7 NULL
9594
1 4 2
95+
2 3 3
9696

9797
query III
9898
select * from t order by v1 desc limit 7;
9999
----
100+
NULL 7 NULL
100101
4 3 5
101102
3 4 4
102103
2 3 3
103104
1 4 2
104-
NULL 7 NULL
105105

106106
statement ok
107107
drop table t;

e2e_test/batch/basic/subquery.slt.part

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,12 @@ select * from t1 where exists(select * from t2 where t1.x = t2.x and t1.y <> t2.
2727
query III
2828
select * from t1 where not exists(select * from t2 where t1.x = t2.x and t1.y <> t2.y) order by t1.x, t1.y
2929
----
30-
NULL NULL
31-
NULL 1
32-
NULL 2
3330
1 NULL
34-
2 NULL
3531
2 2
32+
2 NULL
33+
NULL 1
34+
NULL 2
35+
NULL NULL
3636

3737
query III
3838
select * from t1 where t1.y in (select t1.y from t2 where t1.x = t2.x) order by t1.x, t1.y
@@ -43,16 +43,16 @@ select * from t1 where t1.y in (select t1.y from t2 where t1.x = t2.x) order by
4343
query III
4444
select * from t1 where exists(select x from t2 where t1.x = t2.x and t2.y in (select t3.y from t3 where t1.x = t3.x)) order by t1.x, t1.y
4545
----
46-
1 NULL
4746
1 1
48-
2 NULL
47+
1 NULL
4948
2 2
49+
2 NULL
5050

5151
query III
5252
select * from t1 where exists(select t2.x from t2 join t3 on t2.x = t3.x and t1.y = t2.y and t1.y = t3.y) order by t1.x, t1.y
5353
----
54-
NULL 2
5554
2 2
55+
NULL 2
5656

5757

5858
statement ok
@@ -62,4 +62,4 @@ statement ok
6262
drop table t2;
6363

6464
statement ok
65-
drop table t3;
65+
drop table t3;

src/batch/src/executor/join/lookup_join.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -835,7 +835,6 @@ mod tests {
835835
async fn test_left_outer_join() {
836836
let expected = DataChunk::from_pretty(
837837
"i f i f
838-
. . . .
839838
1 6.1 1 9.2
840839
2 5.5 2 4.4
841840
2 5.5 2 5.5
@@ -845,7 +844,8 @@ mod tests {
845844
5 4.1 5 3.7
846845
5 4.1 5 2.3
847846
5 9.1 5 3.7
848-
5 9.1 5 2.3",
847+
5 9.1 5 2.3
848+
. . . .",
849849
);
850850

851851
do_test(JoinType::LeftOuter, None, expected).await;
@@ -869,8 +869,8 @@ mod tests {
869869
async fn test_left_anti_join() {
870870
let expected = DataChunk::from_pretty(
871871
"i f
872-
. .
873-
3 3.9",
872+
3 3.9
873+
. .",
874874
);
875875

876876
do_test(JoinType::LeftAnti, None, expected).await;
@@ -902,13 +902,13 @@ mod tests {
902902
async fn test_left_outer_join_with_condition() {
903903
let expected = DataChunk::from_pretty(
904904
"i f i f
905-
. . . .
906905
1 6.1 1 9.2
907906
2 5.5 2 5.5
908907
2 8.4 2 5.5
909908
3 3.9 . .
910909
5 4.1 . .
911-
5 9.1 . .",
910+
5 9.1 . .
911+
. . . .",
912912
);
913913

914914
let condition = Some(new_binary_expr(
@@ -950,10 +950,10 @@ mod tests {
950950
async fn test_left_anti_join_with_condition() {
951951
let expected = DataChunk::from_pretty(
952952
"i f
953-
. .
954953
3 3.9
955954
5 4.1
956-
5 9.1",
955+
5 9.1
956+
. .",
957957
);
958958

959959
let condition = Some(new_binary_expr(

src/common/src/types/mod.rs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -359,11 +359,12 @@ pub fn serialize_datum_ref_into(
359359
datum_ref: &DatumRef,
360360
serializer: &mut memcomparable::Serializer<impl BufMut>,
361361
) -> memcomparable::Result<()> {
362+
// By default, `null` is treated as largest in PostgreSQL.
362363
if let Some(datum_ref) = datum_ref {
363-
1u8.serialize(&mut *serializer)?;
364+
0u8.serialize(&mut *serializer)?;
364365
datum_ref.serialize(serializer)?;
365366
} else {
366-
0u8.serialize(serializer)?;
367+
1u8.serialize(serializer)?;
367368
}
368369
Ok(())
369370
}
@@ -384,11 +385,12 @@ pub fn serialize_datum_into(
384385
datum: &Datum,
385386
serializer: &mut memcomparable::Serializer<impl BufMut>,
386387
) -> memcomparable::Result<()> {
388+
// By default, `null` is treated as largest in PostgreSQL.
387389
if let Some(datum) = datum {
388-
1u8.serialize(&mut *serializer)?;
390+
0u8.serialize(&mut *serializer)?;
389391
datum.serialize(serializer)?;
390392
} else {
391-
0u8.serialize(serializer)?;
393+
1u8.serialize(serializer)?;
392394
}
393395
Ok(())
394396
}
@@ -411,8 +413,8 @@ pub fn deserialize_datum_from(
411413
) -> memcomparable::Result<Datum> {
412414
let null_tag = u8::deserialize(&mut *deserializer)?;
413415
match null_tag {
414-
0 => Ok(None),
415-
1 => Ok(Some(ScalarImpl::deserialize(ty.clone(), deserializer)?)),
416+
1 => Ok(None),
417+
0 => Ok(Some(ScalarImpl::deserialize(ty.clone(), deserializer)?)),
416418
_ => Err(memcomparable::Error::InvalidTagEncoding(null_tag as _)),
417419
}
418420
}
@@ -757,8 +759,8 @@ impl ScalarImpl {
757759
let base_position = deserializer.position();
758760
let null_tag = u8::deserialize(&mut *deserializer)?;
759761
match null_tag {
760-
0 => {}
761-
1 => {
762+
1 => {}
763+
0 => {
762764
use std::mem::size_of;
763765
let len = match data_type {
764766
DataType::Int16 => size_of::<i16>(),

src/common/src/util/ordered/serde.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -204,8 +204,8 @@ mod tests {
204204
array.sort();
205205
// option 1 byte || number 2 bytes
206206
assert_eq!(array[0][2], !6i16.to_be_bytes()[1]);
207-
assert_eq!(&array[1][3..], [1, 1, b'a', b'b', b'c', 0, 0, 0, 0, 0, 3u8]);
208-
assert_eq!(&array[2][3..], [1, 1, b'a', b'b', b'd', 0, 0, 0, 0, 0, 3u8]);
207+
assert_eq!(&array[1][3..], [0, 1, b'a', b'b', b'c', 0, 0, 0, 0, 0, 3u8]);
208+
assert_eq!(&array[2][3..], [0, 1, b'a', b'b', b'd', 0, 0, 0, 0, 0, 3u8]);
209209
}
210210

211211
#[test]
@@ -249,7 +249,7 @@ mod tests {
249249
assert_eq!(
250250
array[0][..11],
251251
[
252-
!(1u8),
252+
!(0u8),
253253
!(1u8),
254254
!(b'a'),
255255
!(b'b'),
@@ -262,8 +262,8 @@ mod tests {
262262
!(3u8)
263263
]
264264
);
265-
assert_eq!(array[1][11..], [1, 1, b'j', b'm', b'z', 0, 0, 0, 0, 0, 3u8]);
266-
assert_eq!(array[2][11..], [1, 1, b'm', b'j', b'z', 0, 0, 0, 0, 0, 3u8]);
265+
assert_eq!(array[1][11..], [0, 1, b'j', b'm', b'z', 0, 0, 0, 0, 0, 3u8]);
266+
assert_eq!(array[2][11..], [0, 1, b'm', b'j', b'z', 0, 0, 0, 0, 0, 3u8]);
267267
}
268268

269269
#[test]

src/common/src/util/sort_util.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -172,11 +172,8 @@ where
172172
(Some(l), Some(r)) => l.cmp(r),
173173
(None, None) => Ordering::Equal,
174174
// TODO(yuchao): `null first` / `null last` is not supported yet.
175-
// To be consistent with memcomparable (#116) encoding, `null` is treated as less than any
176-
// non-null value. This is contrary to PostgreSQL's default behavior, where `null`
177-
// is treated as largest.
178-
(Some(_), None) => Ordering::Greater,
179-
(None, Some(_)) => Ordering::Less,
175+
(Some(_), None) => Ordering::Less,
176+
(None, Some(_)) => Ordering::Greater,
180177
};
181178
if *order_type == OrderType::Descending {
182179
ord.reverse()

0 commit comments

Comments
 (0)