-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[CALCITE-5241] Implement CHAR function #2878
Changes from all commits
7d48ec7
2719a10
26e9c10
838e985
b8fda69
c6ad750
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -650,7 +650,22 @@ private SqlLibraryOperators() { | |
ReturnTypes.BIGINT_NULLABLE, null, OperandTypes.TIMESTAMP, | ||
SqlFunctionCategory.TIMEDATE); | ||
|
||
@LibraryOperator(libraries = {ORACLE}) | ||
/** | ||
* The "CHAR(bigint)" function; returns the ASCII character having the binary equivalent | ||
* to bigint; If bigint is larger than 256 the result is equivalent to char(bigint % 256). */ | ||
@LibraryOperator(libraries = {MYSQL, SPARK}) | ||
public static final SqlFunction CHAR = | ||
new SqlFunction("CHAR", | ||
SqlKind.OTHER_FUNCTION, | ||
ReturnTypes.CHAR_FORCE_NULLABLE, | ||
null, | ||
OperandTypes.INTEGER, | ||
SqlFunctionCategory.STRING); | ||
|
||
/** | ||
* The "CHR(bigint)" function; returns the UTF-8 character | ||
* having the binary equivalent to bigint. */ | ||
@LibraryOperator(libraries = {ORACLE, POSTGRESQL}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CHR and CHAR need javadoc There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added javadoc. |
||
public static final SqlFunction CHR = | ||
new SqlFunction("CHR", | ||
SqlKind.OTHER_FUNCTION, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -347,6 +347,13 @@ public static SqlCall stripSeparator(SqlCall call) { | |
public static final SqlReturnTypeInference CHAR = | ||
explicit(SqlTypeName.CHAR); | ||
|
||
/** | ||
* Type-inference strategy whereby the result type of a call is a nullable | ||
* CHAR(1). | ||
*/ | ||
public static final SqlReturnTypeInference CHAR_FORCE_NULLABLE = | ||
CHAR.andThen(SqlTypeTransforms.FORCE_NULLABLE); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not 'nullable Char', but 'nullable CHAR(1)' There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changed to it. |
||
/** | ||
* Type-inference strategy whereby the result type of a call is an Integer. | ||
*/ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -339,6 +339,8 @@ public enum BuiltInMethod { | |
UPPER(SqlFunctions.class, "upper", String.class), | ||
LOWER(SqlFunctions.class, "lower", String.class), | ||
ASCII(SqlFunctions.class, "ascii", String.class), | ||
CHAR(SqlFunctions.class, "charN", long.class), | ||
CHR(SqlFunctions.class, "chr", long.class), | ||
REPEAT(SqlFunctions.class, "repeat", String.class, int.class), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should this be and don't you also need There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for your careful review, I changed to them. |
||
SPACE(SqlFunctions.class, "space", int.class), | ||
SOUNDEX(SqlFunctions.class, "soundex", String.class), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1492,9 +1492,8 @@ protected static Calendar getCalendarNotTooNear(int timeUnit) { | |
f.checkScalar("{fn ASCII('ABC')}", "65", "INTEGER NOT NULL"); | ||
f.checkNull("{fn ASCII(cast(null as varchar(1)))}"); | ||
|
||
if (false) { | ||
f.checkScalar("{fn CHAR(code)}", null, ""); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why remove this test? The goal of the change is to implement JDBC '{fn CHAR}' There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, my mistake. |
||
f.checkScalar("{fn CHAR(97)}", "a", "CHAR(1)"); | ||
|
||
f.checkScalar("{fn CONCAT('foo', 'bar')}", "foobar", "CHAR(6) NOT NULL"); | ||
|
||
f.checkScalar("{fn DIFFERENCE('Miller', 'miller')}", "4", | ||
|
@@ -1630,6 +1629,23 @@ protected static Calendar getCalendarNotTooNear(int timeUnit) { | |
|
||
} | ||
|
||
@Test void testChar() { | ||
final SqlOperatorFixture f0 = fixture() | ||
.setFor(SqlLibraryOperators.CHR, VM_FENNEL, VM_JAVA); | ||
f0.checkFails("^char(97)^", | ||
"No match found for function signature CHAR\\(<NUMERIC>\\)", false); | ||
final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.MYSQL); | ||
f.checkScalar("char(null)", isNullValue(), "CHAR(1)"); | ||
f.checkScalar("char(-1)", isNullValue(), "CHAR(1)"); | ||
f.checkScalar("char(97)", "a", "CHAR(1)"); | ||
f.checkScalar("char(48)", "0", "CHAR(1)"); | ||
f.checkScalar("char(0)", String.valueOf('\u0000'), "CHAR(1)"); | ||
f.checkFails("^char(97.1)^", | ||
"Cannot apply 'CHAR' to arguments of type 'CHAR\\(<DECIMAL\\(3, 1\\)>\\)'\\. " | ||
+ "Supported form\\(s\\): 'CHAR\\(<INTEGER>\\)'", | ||
false); | ||
} | ||
|
||
@Test void testChr() { | ||
final SqlOperatorFixture f0 = fixture() | ||
.setFor(SqlLibraryOperators.CHR, VM_FENNEL, VM_JAVA); | ||
|
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.
explain the difference between this and
chr
. It isn't int vs long.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.
also make the javadoc consistent with other javadoc
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.
Yes, it should be bigint.