You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2021/12/18 17:27:42 UTC
[GitHub] [cassandra] mghildiy opened a new pull request #1368: CASSANDRA-17190:Changes to add support for string concatenations through the + operator
mghildiy opened a new pull request #1368:
URL: https://github.com/apache/cassandra/pull/1368
Changes to add support for string concatenations through the + operator
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] mghildiy edited a comment on pull request #1368: WIP: CASSANDRA-17190:Changes to add support for string concatenations through the + operator
Posted by GitBox <gi...@apache.org>.
mghildiy edited a comment on pull request #1368:
URL: https://github.com/apache/cassandra/pull/1368#issuecomment-1001043882
I think there is still some thing I am missing in code.
I modified one of the existing test as:
```
createTable("CREATE TABLE %s (a tinyint, b smallint, c int, d bigint, e float, f double, g varint, h decimal, i text, j text, PRIMARY KEY(a, b, c))");
execute("INSERT INTO %S (a, b, c, d, e, f, g, h, i, j) VALUES (1, 2, 3, 4, 5.5, 6.5, 7, 8.5, 'John', 'Doe')");
// Test additions
UntypedResultSet rs = execute("SELECT a + a, b + a, c + a, d + a, e + a, f + a, g + a, h + a, i + j FROM %s WHERE a = 1 AND b = 2 AND c = 1 + 2");
```
During debugging for above, I observe that Collection<Function> instance returned by FunctionResolver.collectCandidates for i and j doesn't have the function for our desired combination (text/ascii, text/ascii).
**Note**:
In OperationFcts.java, I see NumericOperationFunction, TemporalOperationFunction implemented. Maybe I need to implement a similar class for string types.
On further debugging, I think I need to modify all() method in OperationFcts.java to add the needed operations to list.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] mghildiy edited a comment on pull request #1368: WIP: CASSANDRA-17190:Changes to add support for string concatenations through the + operator
Posted by GitBox <gi...@apache.org>.
mghildiy edited a comment on pull request #1368:
URL: https://github.com/apache/cassandra/pull/1368#issuecomment-1001043882
I think there is still some thing I am missing in code.
I modified one of the existing test as:
```
createTable("CREATE TABLE %s (a tinyint, b smallint, c int, d bigint, e float, f double, g varint, h decimal, i text, j text, PRIMARY KEY(a, b, c))");
execute("INSERT INTO %S (a, b, c, d, e, f, g, h, i, j) VALUES (1, 2, 3, 4, 5.5, 6.5, 7, 8.5, 'John', 'Doe')");
// Test additions
UntypedResultSet rs = execute("SELECT a + a, b + a, c + a, d + a, e + a, f + a, g + a, h + a, i + j FROM %s WHERE a = 1 AND b = 2 AND c = 1 + 2");
```
During debugging for above, I observe that Collection<Function> instance returned by FunctionResolver.collectCandidates for i and j doesn't have the function for our desired combination (text/ascii, text/ascii).
**Note**:
In OperationFcts.java, I see NumericOperationFunction, TemporalOperationFunction implemented. Maybe I need to implement a similar class for string types.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] blerer commented on a change in pull request #1368: CASSANDRA-17190:Changes to add support for string concatenations through the + operator
Posted by GitBox <gi...@apache.org>.
blerer commented on a change in pull request #1368:
URL: https://github.com/apache/cassandra/pull/1368#discussion_r800456782
##########
File path: test/unit/org/apache/cassandra/cql3/functions/OperationFctsTest.java
##########
@@ -33,6 +33,20 @@
public class OperationFctsTest extends CQLTester
{
+
+ @Test
+ public void testStringConcatenation() throws Throwable
+ {
+ createTable("CREATE TABLE %s (a text, b ascii, c text, PRIMARY KEY(a, b, c))");
Review comment:
I guess that what @bereng is suggesting is to add a user type with `createType(firstname text, surname text)` and to do a concat using `column['firstname'] + column['surname']`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] mghildiy commented on a change in pull request #1368: CASSANDRA-17190:Changes to add support for string concatenations through the + operator
Posted by GitBox <gi...@apache.org>.
mghildiy commented on a change in pull request #1368:
URL: https://github.com/apache/cassandra/pull/1368#discussion_r800033448
##########
File path: test/unit/org/apache/cassandra/cql3/functions/OperationFctsTest.java
##########
@@ -33,6 +33,20 @@
public class OperationFctsTest extends CQLTester
{
+
+ @Test
+ public void testStringConcatenation() throws Throwable
+ {
+ createTable("CREATE TABLE %s (a text, b ascii, c text, PRIMARY KEY(a, b, c))");
Review comment:
a pointer in that direction...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] blerer commented on a change in pull request #1368: CASSANDRA-17190:Changes to add support for string concatenations through the + operator
Posted by GitBox <gi...@apache.org>.
blerer commented on a change in pull request #1368:
URL: https://github.com/apache/cassandra/pull/1368#discussion_r776667972
##########
File path: test/unit/org/apache/cassandra/cql3/functions/OperationFctsTest.java
##########
@@ -36,15 +36,15 @@
@Test
public void testSingleOperations() throws Throwable
{
- createTable("CREATE TABLE %s (a tinyint, b smallint, c int, d bigint, e float, f double, g varint, h decimal, PRIMARY KEY(a, b, c))");
- execute("INSERT INTO %S (a, b, c, d, e, f, g, h) VALUES (1, 2, 3, 4, 5.5, 6.5, 7, 8.5)");
+ createTable("CREATE TABLE %s (a tinyint, b smallint, c int, d bigint, e float, f double, g varint, h decimal, i text, j ascii, PRIMARY KEY(a, b, c))");
+ execute("INSERT INTO %S (a, b, c, d, e, f, g, h, i, j) VALUES (1, 2, 3, 4, 5.5, 6.5, 7, 8.5, 'जॉन', 'Doe')");
// Test additions
- assertColumnNames(execute("SELECT a + a, b + a, c + a, d + a, e + a, f + a, g + a, h + a FROM %s WHERE a = 1 AND b = 2 AND c = 1 + 2"),
- "a + a", "b + a", "c + a", "d + a", "e + a", "f + a", "g + a", "h + a");
+ assertColumnNames(execute("SELECT a + a, b + a, c + a, d + a, e + a, f + a, g + a, h + a, i + i, i + j, j + i, j + j FROM %s WHERE a = 1 AND b = 2 AND c = 1 + 2"),
+ "a + a", "b + a", "c + a", "d + a", "e + a", "f + a", "g + a", "h + a", "i + i", "i + j", "j + i", "j + j");
- assertRows(execute("SELECT a + a, b + a, c + a, d + a, e + a, f + a, g + a, h + a FROM %s WHERE a = 1 AND b = 2 AND c = 1 + 2"),
- row((byte) 2, (short) 3, 4, 5L, 6.5F, 7.5, BigInteger.valueOf(8), BigDecimal.valueOf(9.5)));
+ assertRows(execute("SELECT a + a, b + a, c + a, d + a, e + a, f + a, g + a, h + a, i + ' ' + i, i + ' ' + j, j + ' ' + i, j + ' ' + j FROM %s WHERE a = 1 AND b = 2 AND c = 1 + 2"),
+ row((byte) 2, (short) 3, 4, 5L, 6.5F, 7.5, BigInteger.valueOf(8), BigDecimal.valueOf(9.5), "जॉन जॉन", "जॉन Doe", "Doe जॉन", "Doe Doe"));
Review comment:
I would have separated the tests for String concatenation into a separate test method. This test is already pretty heavy with all the numeric types
##########
File path: src/java/org/apache/cassandra/cql3/Constants.java
##########
@@ -44,7 +45,19 @@
public enum Type
{
- STRING,
+ STRING
+ {
+ public AbstractType<?> getPreferedTypeFor(String text)
+ {
+ if(Charset.forName("US-ASCII").newEncoder().canEncode(text))
+ {
+ return AsciiType.instance;
+ } else
Review comment:
nit: The `else` should be on the next line. You can also choose to remove the `else` as it is not needed do to the `return`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] adiroy05 commented on pull request #1368: WIP: CASSANDRA-17190:Changes to add support for string concatenations through the + operator
Posted by GitBox <gi...@apache.org>.
adiroy05 commented on pull request #1368:
URL: https://github.com/apache/cassandra/pull/1368#issuecomment-997244681
i will do it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] mghildiy commented on pull request #1368: CASSANDRA-17190:Changes to add support for string concatenations through the + operator
Posted by GitBox <gi...@apache.org>.
mghildiy commented on pull request #1368:
URL: https://github.com/apache/cassandra/pull/1368#issuecomment-1002036275
This PR is ready for reciew.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] mghildiy edited a comment on pull request #1368: WIP: CASSANDRA-17190:Changes to add support for string concatenations through the + operator
Posted by GitBox <gi...@apache.org>.
mghildiy edited a comment on pull request #1368:
URL: https://github.com/apache/cassandra/pull/1368#issuecomment-1001043882
I think there is still some thing I am missing in code.
I modified one of the existing test as:
```
createTable("CREATE TABLE %s (a tinyint, b smallint, c int, d bigint, e float, f double, g varint, h decimal, i text, j text, PRIMARY KEY(a, b, c))");
execute("INSERT INTO %S (a, b, c, d, e, f, g, h, i, j) VALUES (1, 2, 3, 4, 5.5, 6.5, 7, 8.5, 'John', 'Doe')");
// Test additions
UntypedResultSet rs = execute("SELECT a + a, b + a, c + a, d + a, e + a, f + a, g + a, h + a, i + j FROM %s WHERE a = 1 AND b = 2 AND c = 1 + 2");
```
During debugging for above, I observe that Collection<Function> instance returned by FunctionResolver.collectCandidates for i and j doesn't have the function for our desired combination (text/ascii, text/ascii).
**Note**:
In OperationFcts.java, I see NumericOperationFunction, TemporalOperationFunction implemented. Maybe I need to implement a similar class for string types.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] mghildiy edited a comment on pull request #1368: CASSANDRA-17190:Changes to add support for string concatenations through the + operator
Posted by GitBox <gi...@apache.org>.
mghildiy edited a comment on pull request #1368:
URL: https://github.com/apache/cassandra/pull/1368#issuecomment-1002036275
This PR is ready for review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] blerer commented on a change in pull request #1368: CASSANDRA-17190:Changes to add support for string concatenations through the + operator
Posted by GitBox <gi...@apache.org>.
blerer commented on a change in pull request #1368:
URL: https://github.com/apache/cassandra/pull/1368#discussion_r802509485
##########
File path: test/unit/org/apache/cassandra/cql3/functions/OperationFctsTest.java
##########
@@ -33,6 +33,20 @@
public class OperationFctsTest extends CQLTester
{
+
+ @Test
+ public void testStringConcatenation() throws Throwable
+ {
+ createTable("CREATE TABLE %s (a text, b ascii, c text, PRIMARY KEY(a, b, c))");
Review comment:
@bereng After thinking about it there are no interactions possible between UDT and the new function. By consequence I do not believe that adding a test with some make sense here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] blerer commented on a change in pull request #1368: WIP: CASSANDRA-17190:Changes to add support for string concatenations through the + operator
Posted by GitBox <gi...@apache.org>.
blerer commented on a change in pull request #1368:
URL: https://github.com/apache/cassandra/pull/1368#discussion_r772140067
##########
File path: src/java/org/apache/cassandra/cql3/functions/OperationFcts.java
##########
@@ -105,6 +106,22 @@ protected ByteBuffer executeOnNumerics(NumberType<?> resultType,
{
return resultType.mod(leftType, left, rightType, right);
}
+ },
+ CONCATENATION('+', "_concat")
Review comment:
There is no need to add a new enum. You should simply implemente the `excuteOnStrings` method in `ADD`. The internal name is hidden and does not play any important role.
##########
File path: src/java/org/apache/cassandra/db/marshal/StringType.java
##########
@@ -18,10 +18,29 @@
package org.apache.cassandra.db.marshal;
+import org.apache.cassandra.utils.ByteBufferUtil;
+
+import java.io.UnsupportedEncodingException;
+import java.nio.ByteBuffer;
+import java.nio.charset.Charset;
+
public abstract class StringType extends AbstractType<String>
{
protected StringType(ComparisonType comparisonType)
{
super(comparisonType);
}
+
+ public abstract String charsetName();
+
+ public ByteBuffer concat(StringType leftType,
+ ByteBuffer left,
+ StringType rightType,
+ ByteBuffer right) throws UnsupportedEncodingException
+ {
+ String leftS = new String(left.array(), leftType.charsetName());
+ String rightS = new String(right.array(), rightType.charsetName());
+
+ return ByteBufferUtil.bytes(leftS + rightS);
+ }
Review comment:
{{ByteBuffer}} do not necessarily have array. If they direct buffer that code will not work. To get the `String` value from the `ByteBuffer` you should use: `leftType.compose(left)` to convert a `String` back into a `ByteBuffer` you should use the `decompose` method. ` return decompose(leftS + rightS);`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] mghildiy edited a comment on pull request #1368: WIP: CASSANDRA-17190:Changes to add support for string concatenations through the + operator
Posted by GitBox <gi...@apache.org>.
mghildiy edited a comment on pull request #1368:
URL: https://github.com/apache/cassandra/pull/1368#issuecomment-1001043882
I think there is still some thing I am missing in code.
I modified one of the existing test as:
```
createTable("CREATE TABLE %s (a tinyint, b smallint, c int, d bigint, e float, f double, g varint, h decimal, i text, j text, PRIMARY KEY(a, b, c))");
execute("INSERT INTO %S (a, b, c, d, e, f, g, h, i, j) VALUES (1, 2, 3, 4, 5.5, 6.5, 7, 8.5, 'John', 'Doe')");
// Test additions
UntypedResultSet rs = execute("SELECT a + a, b + a, c + a, d + a, e + a, f + a, g + a, h + a, i + j FROM %s WHERE a = 1 AND b = 2 AND c = 1 + 2");
```
During debugging for above, I observe that Collection<Function> instance returned by FunctionResolver.collectCandidates for i and j doesn't have the function for our desired combination (text/ascii, text/ascii).
**Note**:
In OperationFcts.java, I see NumericOperationFunction, TemporalOperationFunction implemented. Maybe I need to implement a similar class for string types.
On further debugging, I think I need to modify all() method in OperationFcts.java to add the needed operations to list.
**PLEASE IGNORE. GOT IT WORKING.**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a change in pull request #1368: CASSANDRA-17190:Changes to add support for string concatenations through the + operator
Posted by GitBox <gi...@apache.org>.
bereng commented on a change in pull request #1368:
URL: https://github.com/apache/cassandra/pull/1368#discussion_r799225676
##########
File path: test/unit/org/apache/cassandra/cql3/functions/OperationFctsTest.java
##########
@@ -33,6 +33,20 @@
public class OperationFctsTest extends CQLTester
{
+
+ @Test
+ public void testStringConcatenation() throws Throwable
+ {
+ createTable("CREATE TABLE %s (a text, b ascii, c text, PRIMARY KEY(a, b, c))");
Review comment:
I would add one UDT to be on the safe side as sometimes those behave in funny ways.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] blerer closed pull request #1368: CASSANDRA-17190:Changes to add support for string concatenations through the + operator
Posted by GitBox <gi...@apache.org>.
blerer closed pull request #1368:
URL: https://github.com/apache/cassandra/pull/1368
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] mghildiy commented on pull request #1368: WIP: CASSANDRA-17190:Changes to add support for string concatenations through the + operator
Posted by GitBox <gi...@apache.org>.
mghildiy commented on pull request #1368:
URL: https://github.com/apache/cassandra/pull/1368#issuecomment-1001043882
I think there is still some thing I am missing in code.
I modified one of the existing test as:
```
createTable("CREATE TABLE %s (a tinyint, b smallint, c int, d bigint, e float, f double, g varint, h decimal, i text, j text, PRIMARY KEY(a, b, c))");
execute("INSERT INTO %S (a, b, c, d, e, f, g, h, i, j) VALUES (1, 2, 3, 4, 5.5, 6.5, 7, 8.5, 'John', 'Doe')");
// Test additions
UntypedResultSet rs = execute("SELECT a + a, b + a, c + a, d + a, e + a, f + a, g + a, h + a, i + j FROM %s WHERE a = 1 AND b = 2 AND c = 1 + 2");
```
During debugging for above, I observe that Collection<Function> instance returned by FunctionResolver.collectCandidates for i and j doesn't have the function for our desired combination (text/ascii, text/ascii).
**Note**:
In OperationFcts.jaba, I see NumericOperationFunction, TemporalOperationFunction implemented. Maybe I need to implement a similar class for string types.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] mghildiy edited a comment on pull request #1368: WIP: CASSANDRA-17190:Changes to add support for string concatenations through the + operator
Posted by GitBox <gi...@apache.org>.
mghildiy edited a comment on pull request #1368:
URL: https://github.com/apache/cassandra/pull/1368#issuecomment-1001043882
I think there is still some thing I am missing in code.
I modified one of the existing test as:
```
createTable("CREATE TABLE %s (a tinyint, b smallint, c int, d bigint, e float, f double, g varint, h decimal, i text, j text, PRIMARY KEY(a, b, c))");
execute("INSERT INTO %S (a, b, c, d, e, f, g, h, i, j) VALUES (1, 2, 3, 4, 5.5, 6.5, 7, 8.5, 'John', 'Doe')");
// Test additions
UntypedResultSet rs = execute("SELECT a + a, b + a, c + a, d + a, e + a, f + a, g + a, h + a, i + j FROM %s WHERE a = 1 AND b = 2 AND c = 1 + 2");
```
During debugging for above, I observe that Collection<Function> instance returned by FunctionResolver.collectCandidates for i and j doesn't have the function for our desired combination (text/ascii, text/ascii).
**Note**:
In OperationFcts.java, I see NumericOperationFunction, TemporalOperationFunction implemented. Maybe I need to implement a similar class for string types.
I think I need to modify all() method in OperationFcts.java to add the needed operations to list.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a change in pull request #1368: CASSANDRA-17190:Changes to add support for string concatenations through the + operator
Posted by GitBox <gi...@apache.org>.
bereng commented on a change in pull request #1368:
URL: https://github.com/apache/cassandra/pull/1368#discussion_r799223278
##########
File path: src/java/org/apache/cassandra/cql3/functions/OperationFcts.java
##########
@@ -154,6 +164,25 @@ protected ByteBuffer executeOnTemporals(TemporalType<?> type,
throw new UnsupportedOperationException();
}
+ /**
Review comment:
If we add these it would be good it were std javadoc and I would add it to both methods.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] blerer commented on a change in pull request #1368: CASSANDRA-17190:Changes to add support for string concatenations through the + operator
Posted by GitBox <gi...@apache.org>.
blerer commented on a change in pull request #1368:
URL: https://github.com/apache/cassandra/pull/1368#discussion_r802509485
##########
File path: test/unit/org/apache/cassandra/cql3/functions/OperationFctsTest.java
##########
@@ -33,6 +33,20 @@
public class OperationFctsTest extends CQLTester
{
+
+ @Test
+ public void testStringConcatenation() throws Throwable
+ {
+ createTable("CREATE TABLE %s (a text, b ascii, c text, PRIMARY KEY(a, b, c))");
Review comment:
@bereng After thinking about it there are no interactions possible between UDT and the new function. By consequence I do not believe that adding a test with some is necessary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] blerer commented on pull request #1368: CASSANDRA-17190:Changes to add support for string concatenations through the + operator
Posted by GitBox <gi...@apache.org>.
blerer commented on pull request #1368:
URL: https://github.com/apache/cassandra/pull/1368#issuecomment-1036297829
Committed manually
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org