Skip to content

Commit dc5578a

Browse files
MitchellGaleYury-Fridlyand
MitchellGale
and
Yury-Fridlyand
authored
Add support for long value return for CEIL, CEILING and FLOOR math functions (opensearch-project#1205)
* Added long fix for CEIL, CEILING and FLOOR functions using LONG instead of INT for RETURN. Signed-off-by: MitchellGale-BitQuill <[email protected]> * Revert changes to JDBCConnection.java. Signed-off-by: MitchellGale-BitQuill <[email protected]> * Addressed PR comments. Signed-off-by: MitchellGale-BitQuill <[email protected]> * Made fixes to rst files. Signed-off-by: MitchellGale-BitQuill <[email protected]> * Simplify docs according to PR feedback. Signed-off-by: Yury-Fridlyand <[email protected]> Signed-off-by: MitchellGale-BitQuill <[email protected]> Signed-off-by: Yury-Fridlyand <[email protected]> Co-authored-by: Yury-Fridlyand <[email protected]>
1 parent 7630f87 commit dc5578a

File tree

8 files changed

+195
-67
lines changed

8 files changed

+195
-67
lines changed

‎core/src/main/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunction.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -119,16 +119,16 @@ private static DefaultFunctionResolver abs() {
119119
private static DefaultFunctionResolver ceil() {
120120
return FunctionDSL.define(BuiltinFunctionName.CEIL.getName(),
121121
FunctionDSL.impl(
122-
FunctionDSL.nullMissingHandling(v -> new ExprIntegerValue(Math.ceil(v.doubleValue()))),
123-
INTEGER, DOUBLE)
122+
FunctionDSL.nullMissingHandling(v -> new ExprLongValue(Math.ceil(v.doubleValue()))),
123+
LONG, DOUBLE)
124124
);
125125
}
126126

127127
private static DefaultFunctionResolver ceiling() {
128128
return FunctionDSL.define(BuiltinFunctionName.CEILING.getName(),
129129
FunctionDSL.impl(
130-
FunctionDSL.nullMissingHandling(v -> new ExprIntegerValue(Math.ceil(v.doubleValue()))),
131-
INTEGER, DOUBLE)
130+
FunctionDSL.nullMissingHandling(v -> new ExprLongValue(Math.ceil(v.doubleValue()))),
131+
LONG, DOUBLE)
132132
);
133133
}
134134

@@ -204,8 +204,8 @@ private static DefaultFunctionResolver exp() {
204204
private static DefaultFunctionResolver floor() {
205205
return FunctionDSL.define(BuiltinFunctionName.FLOOR.getName(),
206206
FunctionDSL.impl(
207-
FunctionDSL.nullMissingHandling(v -> new ExprIntegerValue(Math.floor(v.doubleValue()))),
208-
INTEGER, DOUBLE)
207+
FunctionDSL.nullMissingHandling(v -> new ExprLongValue(Math.floor(v.doubleValue()))),
208+
LONG, DOUBLE)
209209
);
210210
}
211211

‎core/src/test/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunctionTest.java‎

Lines changed: 43 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -191,13 +191,13 @@ public void ceil_int_value(Integer value) {
191191
FunctionExpression ceil = DSL.ceil(DSL.literal(value));
192192
assertThat(
193193
ceil.valueOf(valueEnv()),
194-
allOf(hasType(INTEGER), hasValue((int) Math.ceil(value))));
195-
assertEquals(String.format("ceil(%s)", value), ceil.toString());
194+
allOf(hasType(LONG), hasValue((long) Math.ceil(value))));
195+
assertEquals(String.format("ceil(%s)", value.toString()), ceil.toString());
196196

197197
FunctionExpression ceiling = DSL.ceiling(DSL.literal(value));
198198
assertThat(
199-
ceiling.valueOf(valueEnv()), allOf(hasType(INTEGER), hasValue((int) Math.ceil(value))));
200-
assertEquals(String.format("ceiling(%s)", value), ceiling.toString());
199+
ceiling.valueOf(valueEnv()), allOf(hasType(LONG), hasValue((long) Math.ceil(value))));
200+
assertEquals(String.format("ceiling(%s)", value.toString()), ceiling.toString());
201201
}
202202

203203
/**
@@ -208,13 +208,30 @@ public void ceil_int_value(Integer value) {
208208
public void ceil_long_value(Long value) {
209209
FunctionExpression ceil = DSL.ceil(DSL.literal(value));
210210
assertThat(
211-
ceil.valueOf(valueEnv()), allOf(hasType(INTEGER), hasValue((int) Math.ceil(value))));
212-
assertEquals(String.format("ceil(%s)", value), ceil.toString());
211+
ceil.valueOf(valueEnv()), allOf(hasType(LONG), hasValue((long) Math.ceil(value))));
212+
assertEquals(String.format("ceil(%s)", value.toString()), ceil.toString());
213213

214214
FunctionExpression ceiling = DSL.ceiling(DSL.literal(value));
215215
assertThat(
216-
ceiling.valueOf(valueEnv()), allOf(hasType(INTEGER), hasValue((int) Math.ceil(value))));
217-
assertEquals(String.format("ceiling(%s)", value), ceiling.toString());
216+
ceiling.valueOf(valueEnv()), allOf(hasType(LONG), hasValue((long) Math.ceil(value))));
217+
assertEquals(String.format("ceiling(%s)", value.toString()), ceiling.toString());
218+
}
219+
220+
/**
221+
* Test ceil/ceiling with long value.
222+
*/
223+
@ParameterizedTest(name = "ceil({0})")
224+
@ValueSource(longs = {9223372036854775805L, -9223372036854775805L})
225+
public void ceil_long_value_long(Long value) {
226+
FunctionExpression ceil = DSL.ceil(DSL.literal(value));
227+
assertThat(
228+
ceil.valueOf(valueEnv()), allOf(hasType(LONG), hasValue((long) Math.ceil(value))));
229+
assertEquals(String.format("ceil(%s)", value.toString()), ceil.toString());
230+
231+
FunctionExpression ceiling = DSL.ceiling(DSL.literal(value));
232+
assertThat(
233+
ceiling.valueOf(valueEnv()), allOf(hasType(LONG), hasValue((long) Math.ceil(value))));
234+
assertEquals(String.format("ceiling(%s)", value.toString()), ceiling.toString());
218235
}
219236

220237
/**
@@ -225,13 +242,13 @@ public void ceil_long_value(Long value) {
225242
public void ceil_float_value(Float value) {
226243
FunctionExpression ceil = DSL.ceil(DSL.literal(value));
227244
assertThat(
228-
ceil.valueOf(valueEnv()), allOf(hasType(INTEGER), hasValue((int) Math.ceil(value))));
229-
assertEquals(String.format("ceil(%s)", value), ceil.toString());
245+
ceil.valueOf(valueEnv()), allOf(hasType(LONG), hasValue((long) Math.ceil(value))));
246+
assertEquals(String.format("ceil(%s)", value.toString()), ceil.toString());
230247

231248
FunctionExpression ceiling = DSL.ceiling(DSL.literal(value));
232249
assertThat(
233-
ceiling.valueOf(valueEnv()), allOf(hasType(INTEGER), hasValue((int) Math.ceil(value))));
234-
assertEquals(String.format("ceiling(%s)", value), ceiling.toString());
250+
ceiling.valueOf(valueEnv()), allOf(hasType(LONG), hasValue((long) Math.ceil(value))));
251+
assertEquals(String.format("ceiling(%s)", value.toString()), ceiling.toString());
235252
}
236253

237254
/**
@@ -242,13 +259,13 @@ public void ceil_float_value(Float value) {
242259
public void ceil_double_value(Double value) {
243260
FunctionExpression ceil = DSL.ceil(DSL.literal(value));
244261
assertThat(
245-
ceil.valueOf(valueEnv()), allOf(hasType(INTEGER), hasValue((int) Math.ceil(value))));
246-
assertEquals(String.format("ceil(%s)", value), ceil.toString());
262+
ceil.valueOf(valueEnv()), allOf(hasType(LONG), hasValue((long) Math.ceil(value))));
263+
assertEquals(String.format("ceil(%s)", value.toString()), ceil.toString());
247264

248265
FunctionExpression ceiling = DSL.ceiling(DSL.literal(value));
249266
assertThat(
250-
ceiling.valueOf(valueEnv()), allOf(hasType(INTEGER), hasValue((int) Math.ceil(value))));
251-
assertEquals(String.format("ceiling(%s)", value), ceiling.toString());
267+
ceiling.valueOf(valueEnv()), allOf(hasType(LONG), hasValue((long) Math.ceil(value))));
268+
assertEquals(String.format("ceiling(%s)", value.toString()), ceiling.toString());
252269
}
253270

254271
/**
@@ -257,11 +274,11 @@ public void ceil_double_value(Double value) {
257274
@Test
258275
public void ceil_null_value() {
259276
FunctionExpression ceil = DSL.ceil(DSL.ref(DOUBLE_TYPE_NULL_VALUE_FIELD, DOUBLE));
260-
assertEquals(INTEGER, ceil.type());
277+
assertEquals(LONG, ceil.type());
261278
assertTrue(ceil.valueOf(valueEnv()).isNull());
262279

263280
FunctionExpression ceiling = DSL.ceiling(DSL.ref(DOUBLE_TYPE_NULL_VALUE_FIELD, DOUBLE));
264-
assertEquals(INTEGER, ceiling.type());
281+
assertEquals(LONG, ceiling.type());
265282
assertTrue(ceiling.valueOf(valueEnv()).isNull());
266283
}
267284

@@ -271,11 +288,11 @@ public void ceil_null_value() {
271288
@Test
272289
public void ceil_missing_value() {
273290
FunctionExpression ceil = DSL.ceil(DSL.ref(DOUBLE_TYPE_MISSING_VALUE_FIELD, DOUBLE));
274-
assertEquals(INTEGER, ceil.type());
291+
assertEquals(LONG, ceil.type());
275292
assertTrue(ceil.valueOf(valueEnv()).isMissing());
276293

277294
FunctionExpression ceiling = DSL.ceiling(DSL.ref(DOUBLE_TYPE_MISSING_VALUE_FIELD, DOUBLE));
278-
assertEquals(INTEGER, ceiling.type());
295+
assertEquals(LONG, ceiling.type());
279296
assertTrue(ceiling.valueOf(valueEnv()).isMissing());
280297
}
281298

@@ -557,7 +574,7 @@ public void floor_int_value(Integer value) {
557574
FunctionExpression floor = DSL.floor(DSL.literal(value));
558575
assertThat(
559576
floor.valueOf(valueEnv()),
560-
allOf(hasType(INTEGER), hasValue((int) Math.floor(value))));
577+
allOf(hasType(LONG), hasValue((long) Math.floor(value))));
561578
assertEquals(String.format("floor(%s)", value.toString()), floor.toString());
562579
}
563580

@@ -570,7 +587,7 @@ public void floor_long_value(Long value) {
570587
FunctionExpression floor = DSL.floor(DSL.literal(value));
571588
assertThat(
572589
floor.valueOf(valueEnv()),
573-
allOf(hasType(INTEGER), hasValue((int) Math.floor(value))));
590+
allOf(hasType(LONG), hasValue((long) Math.floor(value))));
574591
assertEquals(String.format("floor(%s)", value.toString()), floor.toString());
575592
}
576593

@@ -583,7 +600,7 @@ public void floor_float_value(Float value) {
583600
FunctionExpression floor = DSL.floor(DSL.literal(value));
584601
assertThat(
585602
floor.valueOf(valueEnv()),
586-
allOf(hasType(INTEGER), hasValue((int) Math.floor(value))));
603+
allOf(hasType(LONG), hasValue((long) Math.floor(value))));
587604
assertEquals(String.format("floor(%s)", value.toString()), floor.toString());
588605
}
589606

@@ -596,7 +613,7 @@ public void floor_double_value(Double value) {
596613
FunctionExpression floor = DSL.floor(DSL.literal(value));
597614
assertThat(
598615
floor.valueOf(valueEnv()),
599-
allOf(hasType(INTEGER), hasValue((int) Math.floor(value))));
616+
allOf(hasType(LONG), hasValue((long) Math.floor(value))));
600617
assertEquals(String.format("floor(%s)", value.toString()), floor.toString());
601618
}
602619

@@ -606,7 +623,7 @@ public void floor_double_value(Double value) {
606623
@Test
607624
public void floor_null_value() {
608625
FunctionExpression floor = DSL.floor(DSL.ref(DOUBLE_TYPE_NULL_VALUE_FIELD, DOUBLE));
609-
assertEquals(INTEGER, floor.type());
626+
assertEquals(LONG, floor.type());
610627
assertTrue(floor.valueOf(valueEnv()).isNull());
611628
}
612629

@@ -616,7 +633,7 @@ public void floor_null_value() {
616633
@Test
617634
public void floor_missing_value() {
618635
FunctionExpression floor = DSL.floor(DSL.ref(DOUBLE_TYPE_MISSING_VALUE_FIELD, DOUBLE));
619-
assertEquals(INTEGER, floor.type());
636+
assertEquals(LONG, floor.type());
620637
assertTrue(floor.valueOf(valueEnv()).isMissing());
621638
}
622639

‎docs/user/dql/functions.rst‎

Lines changed: 63 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -231,12 +231,42 @@ Example::
231231
CEIL
232232
----
233233

234+
An alias for `CEILING`_ function.
235+
236+
237+
CEILING
238+
-------
239+
234240
Description
235241
>>>>>>>>>>>
236242

237-
Specifications:
243+
Usage: CEILING(T) takes the ceiling of value T.
244+
245+
Note: `CEIL`_ and CEILING functions have the same implementation & functionality
246+
247+
Limitation: CEILING only works as expected when IEEE 754 double type displays decimal when stored.
248+
249+
Argument type: INTEGER/LONG/FLOAT/DOUBLE
250+
251+
Return type: LONG
252+
253+
Example::
254+
255+
os> SELECT CEILING(0), CEILING(50.00005), CEILING(-50.00005);
256+
fetched rows / total rows = 1/1
257+
+--------------+---------------------+----------------------+
258+
| CEILING(0) | CEILING(50.00005) | CEILING(-50.00005) |
259+
|--------------+---------------------+----------------------|
260+
| 0 | 51 | -50 |
261+
+--------------+---------------------+----------------------+
238262

239-
1. CEIL(NUMBER T) -> T
263+
os> SELECT CEILING(3147483647.12345), CEILING(113147483647.12345), CEILING(3147483647.00001);
264+
fetched rows / total rows = 1/1
265+
+-----------------------------+-------------------------------+-----------------------------+
266+
| CEILING(3147483647.12345) | CEILING(113147483647.12345) | CEILING(3147483647.00001) |
267+
|-----------------------------+-------------------------------+-----------------------------|
268+
| 3147483648 | 113147483648 | 3147483648 |
269+
+-----------------------------+-------------------------------+-----------------------------+
240270

241271

242272
CONV
@@ -424,10 +454,39 @@ FLOOR
424454
Description
425455
>>>>>>>>>>>
426456

427-
Specifications:
457+
Usage: FLOOR(T) takes the floor of value T.
458+
459+
Limitation: FLOOR only works as expected when IEEE 754 double type displays decimal when stored.
460+
461+
Argument type: INTEGER/LONG/FLOAT/DOUBLE
462+
463+
Return type: LONG
464+
465+
Example::
428466

429-
1. FLOOR(NUMBER T) -> T
467+
os> SELECT FLOOR(0), FLOOR(50.00005), FLOOR(-50.00005);
468+
fetched rows / total rows = 1/1
469+
+------------+-------------------+--------------------+
470+
| FLOOR(0) | FLOOR(50.00005) | FLOOR(-50.00005) |
471+
|------------+-------------------+--------------------|
472+
| 0 | 50 | -51 |
473+
+------------+-------------------+--------------------+
430474

475+
os> SELECT FLOOR(3147483647.12345), FLOOR(113147483647.12345), FLOOR(3147483647.00001);
476+
fetched rows / total rows = 1/1
477+
+---------------------------+-----------------------------+---------------------------+
478+
| FLOOR(3147483647.12345) | FLOOR(113147483647.12345) | FLOOR(3147483647.00001) |
479+
|---------------------------+-----------------------------+---------------------------|
480+
| 3147483647 | 113147483647 | 3147483647 |
481+
+---------------------------+-----------------------------+---------------------------+
482+
483+
os> SELECT FLOOR(282474973688888.022), FLOOR(9223372036854775807.022), FLOOR(9223372036854775807.0000001);
484+
fetched rows / total rows = 1/1
485+
+------------------------------+----------------------------------+--------------------------------------+
486+
| FLOOR(282474973688888.022) | FLOOR(9223372036854775807.022) | FLOOR(9223372036854775807.0000001) |
487+
|------------------------------+----------------------------------+--------------------------------------|
488+
| 282474973688888 | 9223372036854775807 | 9223372036854775807 |
489+
+------------------------------+----------------------------------+--------------------------------------+
431490

432491
LN
433492
--

‎docs/user/ppl/functions/math.rst‎

Lines changed: 52 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -127,24 +127,42 @@ Example::
127127
CEIL
128128
----
129129

130+
An alias for `CEILING`_ function.
131+
132+
133+
CEILING
134+
-------
135+
130136
Description
131137
>>>>>>>>>>>
132138

133-
Usage: ceil(x) return the smallest integer value this is greater or equal to x.
139+
Usage: CEILING(T) takes the ceiling of value T.
140+
141+
Note: `CEIL`_ and CEILING functions have the same implementation & functionality
142+
143+
Limitation: CEILING only works as expected when IEEE 754 double type displays decimal when stored.
134144

135145
Argument type: INTEGER/LONG/FLOAT/DOUBLE
136146

137-
Return type: INTEGER
147+
Return type: LONG
138148

139149
Example::
140150

141-
os> source=people | eval `CEIL(2.75)` = CEIL(2.75) | fields `CEIL(2.75)`
151+
os> source=people | eval `CEILING(0)` = CEILING(0), `CEILING(50.00005)` = CEILING(50.00005), `CEILING(-50.00005)` = CEILING(-50.00005) | fields `CEILING(0)`, `CEILING(50.00005)`, `CEILING(-50.00005)`
142152
fetched rows / total rows = 1/1
143-
+--------------+
144-
| CEIL(2.75) |
145-
|--------------|
146-
| 3 |
147-
+--------------+
153+
+--------------+---------------------+----------------------+
154+
| CEILING(0) | CEILING(50.00005) | CEILING(-50.00005) |
155+
|--------------+---------------------+----------------------|
156+
| 0 | 51 | -50 |
157+
+--------------+---------------------+----------------------+
158+
159+
os> source=people | eval `CEILING(3147483647.12345)` = CEILING(3147483647.12345), `CEILING(113147483647.12345)` = CEILING(113147483647.12345), `CEILING(3147483647.00001)` = CEILING(3147483647.00001) | fields `CEILING(3147483647.12345)`, `CEILING(113147483647.12345)`, `CEILING(3147483647.00001)`
160+
fetched rows / total rows = 1/1
161+
+-----------------------------+-------------------------------+-----------------------------+
162+
| CEILING(3147483647.12345) | CEILING(113147483647.12345) | CEILING(3147483647.00001) |
163+
|-----------------------------+-------------------------------+-----------------------------|
164+
| 3147483648 | 113147483648 | 3147483648 |
165+
+-----------------------------+-------------------------------+-----------------------------+
148166

149167

150168
CONV
@@ -310,22 +328,39 @@ FLOOR
310328
Description
311329
>>>>>>>>>>>
312330

313-
Usage: floor(x) return the largest integer value this is smaller or equal to x.
331+
Usage: FLOOR(T) takes the floor of value T.
314332

315-
Argument type: INTEGER/LONG/FLOAT/DOUBLE
333+
Limitation: FLOOR only works as expected when IEEE 754 double type displays decimal when stored.
316334

317-
Return type: INTEGER
335+
Argument type: a: INTEGER/LONG/FLOAT/DOUBLE
336+
337+
Return type: LONG
318338

319339
Example::
320340

321-
os> source=people | eval `FLOOR(2.75)` = FLOOR(2.75) | fields `FLOOR(2.75)`
341+
os> source=people | eval `FLOOR(0)` = FLOOR(0), `FLOOR(50.00005)` = FLOOR(50.00005), `FLOOR(-50.00005)` = FLOOR(-50.00005) | fields `FLOOR(0)`, `FLOOR(50.00005)`, `FLOOR(-50.00005)`
322342
fetched rows / total rows = 1/1
323-
+---------------+
324-
| FLOOR(2.75) |
325-
|---------------|
326-
| 2 |
327-
+---------------+
343+
+------------+-------------------+--------------------+
344+
| FLOOR(0) | FLOOR(50.00005) | FLOOR(-50.00005) |
345+
|------------+-------------------+--------------------|
346+
| 0 | 50 | -51 |
347+
+------------+-------------------+--------------------+
328348

349+
os> source=people | eval `FLOOR(3147483647.12345)` = FLOOR(3147483647.12345), `FLOOR(113147483647.12345)` = FLOOR(113147483647.12345), `FLOOR(3147483647.00001)` = FLOOR(3147483647.00001) | fields `FLOOR(3147483647.12345)`, `FLOOR(113147483647.12345)`, `FLOOR(3147483647.00001)`
350+
fetched rows / total rows = 1/1
351+
+---------------------------+-----------------------------+---------------------------+
352+
| FLOOR(3147483647.12345) | FLOOR(113147483647.12345) | FLOOR(3147483647.00001) |
353+
|---------------------------+-----------------------------+---------------------------|
354+
| 3147483647 | 113147483647 | 3147483647 |
355+
+---------------------------+-----------------------------+---------------------------+
356+
357+
os> source=people | eval `FLOOR(282474973688888.022)` = FLOOR(282474973688888.022), `FLOOR(9223372036854775807.022)` = FLOOR(9223372036854775807.022), `FLOOR(9223372036854775807.0000001)` = FLOOR(9223372036854775807.0000001) | fields `FLOOR(282474973688888.022)`, `FLOOR(9223372036854775807.022)`, `FLOOR(9223372036854775807.0000001)`
358+
fetched rows / total rows = 1/1
359+
+------------------------------+----------------------------------+--------------------------------------+
360+
| FLOOR(282474973688888.022) | FLOOR(9223372036854775807.022) | FLOOR(9223372036854775807.0000001) |
361+
|------------------------------+----------------------------------+--------------------------------------|
362+
| 282474973688888 | 9223372036854775807 | 9223372036854775807 |
363+
+------------------------------+----------------------------------+--------------------------------------+
329364

330365
LN
331366
--

‎integ-test/src/test/java/org/opensearch/sql/legacy/TypeInformationIT.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public void testCeilWithLongFieldReturnsLong() {
4040
executeJdbcRequest("SELECT CEIL(balance) FROM " + TestsConstants.TEST_INDEX_ACCOUNT +
4141
" ORDER BY balance LIMIT 5");
4242

43-
verifySchema(response, schema("CEIL(balance)", null, "integer"));
43+
verifySchema(response, schema("CEIL(balance)", null, "long"));
4444
}
4545

4646
/*

0 commit comments

Comments
 (0)