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.

Sign up for GitHub

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

Jump to bottom

Fix scalar exp encoding #665

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Fix scalar exp encoding #665

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
cc80f2f
Fix string scalar encoding
m-dzianishchyts Nov 17, 2023
9f686f4
Fix numeric scalar encoding
m-dzianishchyts Jan 31, 2024
5f90a37
Merge branch 'master' into exp-scalar-encoding
m-dzianishchyts Jan 31, 2024
92c4ba6
Revert "Fix string scalar encoding"
m-dzianishchyts Feb 1, 2024
dc71bf7
Store quoted string in scalarImage
m-dzianishchyts Feb 1, 2024
eed112d
Refactor 'scalarImage' initialization
m-dzianishchyts Feb 3, 2024
fea382c
Apply suggestions from code review
m-dzianishchyts Feb 6, 2024
28422c6
Merge branch 'master' into exp-scalar-encoding
m-dzianishchyts Feb 8, 2024
84ee2a6
Fix exp-string conversion for scalar
m-dzianishchyts Feb 8, 2024
b63dcf0
Clean up
m-dzianishchyts Feb 8, 2024
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
22 changes: 12 additions & 10 deletions agrest-engine/src/main/java/io/agrest/exp/parser/AgExpressionParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -967,32 +967,34 @@ final public void stringLiteral() throws ParseException {
case SINGLE_QUOTED_STRING:{
jj_consume_token(SINGLE_QUOTED_STRING);
ExpScalar jjtn001 = new ExpScalar(JJTSCALAR);
boolean jjtc001 = true;
jjtree.openNodeScope(jjtn001);
boolean jjtc001 = true;
jjtree.openNodeScope(jjtn001);
try {
jjtree.closeNodeScope(jjtn001, 0);
jjtc001 = false;
jjtc001 = false;
jjtn001.jjtSetValue(token_source.literalValue);
jjtn001.syncScalarImage("'" + token_source.literalValue + "'");
} finally {
if (jjtc001) {
jjtree.closeNodeScope(jjtn001, 0);
}
jjtree.closeNodeScope(jjtn001, 0);
}
}
break;
}
case DOUBLE_QUOTED_STRING:{
jj_consume_token(DOUBLE_QUOTED_STRING);
ExpScalar jjtn002 = new ExpScalar(JJTSCALAR);
boolean jjtc002 = true;
jjtree.openNodeScope(jjtn002);
boolean jjtc002 = true;
jjtree.openNodeScope(jjtn002);
try {
jjtree.closeNodeScope(jjtn002, 0);
jjtc002 = false;
jjtc002 = false;
jjtn002.jjtSetValue(token_source.literalValue);
jjtn002.syncScalarImage("\"" + token_source.literalValue + "\"");
} finally {
if (jjtc002) {
jjtree.closeNodeScope(jjtn002, 0);
}
jjtree.closeNodeScope(jjtn002, 0);
}
}
break;
}
Expand Down
73 changes: 67 additions & 6 deletions agrest-engine/src/main/java/io/agrest/exp/parser/ExpScalar.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,22 @@
/* JavaCCOptions:MULTI=true,NODE_USES_PARSER=false,VISITOR=true,TRACK_TOKENS=false,NODE_PREFIX=Exp,NODE_EXTENDS=,NODE_FACTORY=,SUPPORT_CLASS_VISIBILITY_PUBLIC=true */
package io.agrest.exp.parser;

import java.math.BigDecimal;
import java.math.BigInteger;
import java.util.function.Function;

public class ExpScalar extends ExpBaseScalar<Object> {

private String scalarImage;
m-dzianishchyts marked this conversation as resolved.
Show resolved Hide resolved

public ExpScalar(int id) {
super(id);
syncScalarImage(value);
m-dzianishchyts marked this conversation as resolved.
Show resolved Hide resolved
}

public ExpScalar(AgExpressionParser p, int id) {
super(p, id);
syncScalarImage(value);
m-dzianishchyts marked this conversation as resolved.
Show resolved Hide resolved
}

public ExpScalar() {
Expand All @@ -27,26 +34,80 @@ protected Object transformExpression(Function<Object, Object> transformer) {
return transformer.apply(new ExpScalar(getValue()));
}

protected void syncScalarImage(CharSequence value) {
if (value.length() < 2) {
scalarImage = "'" + value + "'";
return;
}
String stringValue = value.toString();
char firstChar = stringValue.charAt(0);
if (firstChar != '\'' && firstChar != '"') {
scalarImage = "'" + value + "'";
return;
}
String escapedContent = stringValue
.substring(1, stringValue.length() - 1)
.replaceAll(String.valueOf(firstChar), "\\\\" + firstChar);
scalarImage = firstChar + escapedContent + firstChar;
}

protected void syncScalarImage(Long value) {
scalarImage = value + "L";
}

protected void syncScalarImage(BigInteger value) {
scalarImage = value + "H";
}

protected void syncScalarImage(BigDecimal value) {
scalarImage = value + "B";
}

protected void syncScalarImage(Object value) {
scalarImage = String.valueOf(value);
}

/**
* Accept the visitor.
**/
public <T> T jjtAccept(AgExpressionParserVisitor<T> visitor, T data) {

return visitor.visit(this, data);
}

@Override
public void jjtSetValue(Object value) {
super.jjtSetValue(value);
if (value == null) {
scalarImage = "null";
return;
}
if (value instanceof CharSequence) {
syncScalarImage(((CharSequence) value));
return;
}
if (value.getClass() == Long.class) {
m-dzianishchyts marked this conversation as resolved.
Show resolved Hide resolved
syncScalarImage(((Long) value));
return;
}
if (value.getClass() == BigInteger.class) {
syncScalarImage(((BigInteger) value));
return;
}
if (value.getClass() == BigDecimal.class) {
syncScalarImage(((BigDecimal) value));
return;
}
syncScalarImage(value);
}

@Override
protected ExpScalar shallowCopy() {
return new ExpScalar(getValue());
}

@Override
public String toString() {
if (value instanceof CharSequence) {
return "'" + value + "'";
} else {
return String.valueOf(value);
}
return scalarImage;
}
}
/* JavaCC - OriginalChecksum=21004db13a44c6b16cc9797a6f36a4af (do not edit this line) */
17 changes: 9 additions & 8 deletions agrest-engine/src/main/java/io/agrest/protocol/Exp.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,26 +64,27 @@ static Exp path(String path) {
*/
static Exp scalar(Object value) {
if (value == null) {
return new ExpScalar(AgExpressionParserTreeConstants.JJTSCALAR);
return new ExpScalar();
}

ExpBaseScalar<?> scalar;
if (value instanceof Collection) {
scalar = new ExpScalarList(AgExpressionParserTreeConstants.JJTSCALARLIST);
} else if (value.getClass().isArray()) {
scalar = new ExpScalarList((Collection<?>) value);
return scalar;
}
if (value.getClass().isArray()) {
Class<?> componentType = value.getClass().getComponentType();
if (componentType.isPrimitive()) {
value = ExpUtils.wrapPrimitiveArray(value);
} else {
value = Arrays.asList((Object[]) value);
}
scalar = new ExpScalarList(AgExpressionParserTreeConstants.JJTSCALARLIST);
} else {
scalar = new ExpScalar(AgExpressionParserTreeConstants.JJTSCALAR);
scalar = new ExpScalarList();
scalar.jjtSetValue(value);
return scalar;
}

scalar.jjtSetValue(value);
return scalar;
return new ExpScalar(value);
}

/**
Expand Down
12 changes: 10 additions & 2 deletions agrest-engine/src/main/jjtree/io/agrest/exp/parser/AgExpressionParser.jjt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,17 @@ void stringParameter() : {}

void stringLiteral() : {}
{
<SINGLE_QUOTED_STRING> { jjtThis.jjtSetValue(token_source.literalValue); } #Scalar(0)
<SINGLE_QUOTED_STRING>
{
jjtThis.jjtSetValue(token_source.literalValue);
jjtThis.syncScalarImage("'" + token_source.literalValue + "'");
} #Scalar(0)
|
<DOUBLE_QUOTED_STRING> { jjtThis.jjtSetValue(token_source.literalValue); } #Scalar(0)
<DOUBLE_QUOTED_STRING>
{
jjtThis.jjtSetValue(token_source.literalValue);
jjtThis.syncScalarImage("\"" + token_source.literalValue + "\"");
} #Scalar(0)
}

void stringExpression() : {}
Expand Down
10 changes: 4 additions & 6 deletions agrest-engine/src/test/java/io/agrest/exp/parser/ExpScalarFloatTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,13 @@ void parse(String expString) {
"1e1|10.0",
"1E1|10.0",
"1.1e1|11.0",
"3.4028235e+38f|3.4028235E38",
"1.7976931348623157e+308|1.7976931348623157E308",
"1.7976931348623157e+308d|1.7976931348623157E308",
"1_2.1|12.1",
"2.2__34|2.234",
".3e7_6|3.0E75",

// TODO: this is wrong, B suffix expected
"1.7976931348623157e+309b|1.7976931348623157E+309"
"3.4028235e+38f|3.4028235E38",
"1.7976931348623157e+308|1.7976931348623157E308",
"1.7976931348623157e+308d|1.7976931348623157E308",
"1.7976931348623157e+309b|1.7976931348623157E+309B"
})
public void parsedToString(String expString, String expected) {
assertEquals(expected, Exp.parse(expString).toString());
Expand Down
6 changes: 2 additions & 4 deletions agrest-engine/src/test/java/io/agrest/exp/parser/ExpScalarIntTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,8 @@ void parse(String expString) {
"01_2|10",
"0x1_A|26",
"123___456|123456",

// TODO: this is wrong, L suffix expected in the output
"2147483648L|2147483648",
"9223372036854775808H|9223372036854775808",
"2147483648L|2147483648L",
"9223372036854775808H|9223372036854775808H",
"01234567|342391",
"0x12345678|305419896",
"0x09abcdef|162254319"
Expand Down
9 changes: 4 additions & 5 deletions agrest-engine/src/test/java/io/agrest/exp/parser/ExpScalarStringTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,12 @@ void parse(String expString) {
@CsvSource(delimiter = '|', quoteCharacter = 'X', value = {
"'example'|'example'",
" 'example' |'example'",
"\"example\"|'example'",
"\"example\"|\"example\"",
"''|''",
"' '|' '",
"\"\\\"example\\\"\"|'\"example\"'",
// TODO: this is wrong, single quote must be escaped or double quotes used in output
"\"a'b\"|'a'b'",
"'a\"b'|'a\"b'"
"\"\\\"example\\\"\"|\"\\\"example\\\"\"",
"\"a\\\"'b\"|\"a\\\"'b\"",
"'a\"\\'b'|'a\"\\'b'"
})
public void parsedToString(String expString, String expected) {
assertEquals(expected, Exp.parse(expString).toString());
Expand Down
Loading