[
https://issues.apache.org/jira/browse/PHOENIX-1705?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14377461#comment-14377461
]
James Taylor commented on PHOENIX-1705:
---------------------------------------
Looking good, [~Dumindux]. Here's some feedback:
- The array_append function should return the same type as the first argument,
something like this instead:
{code}
+ @Override
+ public PDataType getDataType() {
+ return children.get(0).getDataType();
+ }
+
{code}
- Use PDataType.arrayBaseType(PDataType arrayType) function instead of doing
the calculation here yourself:
{code}
+ public ArrayAppendFunction(List<Expression> children) throws
TypeMismatchException {
+
+ super(children);
+
baseType=PDataType.fromTypeId(children.get(0).getDataType().getSqlType()
+ - PDataType.ARRAY_TYPE_BASE);
+ elementDataType=children.get(1).getDataType();
+
+ if(elementDataType != null && !elementDataType.equals(baseType)){
+ if(!elementDataType.isCastableTo(baseType)){
+ throw
TypeMismatchException.newException(baseType,elementDataType);
+ }
+ }
+
+ }
+
{code}
- Minor formatting issue, but keep the starting curly brace on the same line as
the statement:
{code}
+ if (baseType.isFixedWidth()) // move curly brace here
+ {
{code}
- Remove all System.out.println calls:
{code}
+ @Test
+ public void testArrayAppendWithNestedFunctions2() throws Exception {
+ Connection conn = DriverManager.getConnection(getUrl());
+ initTables(conn);
+
+ ResultSet rs;
+ rs = conn.createStatement().executeQuery("SELECT
ARRAY_APPEND(integers,ARRAY_ELEM(ARRAY[2,4],1)) FROM regions WHERE region_name
= 'SF Bay Area'");
+ assertTrue(rs.next());
+
+ Integer[] integers = new Integer[]{2345,46345,23234,456,2};
+
+ Array array = conn.createArrayOf("INTEGER", integers);
+
+ System.out.println("arrr "+ rs.getArray(1).toString());
+ assertEquals(array, rs.getArray(1));
+ assertFalse(rs.next());
+ }
{code}
- For your evaluate function, return true immediately if the element to be
appended is null, as we don't store trailing nulls in an array.
{code}
+ @Override
+ public boolean evaluate(Tuple tuple, ImmutableBytesWritable ptr) {
+
+ Expression elementExpr = children.get(1);
+ Expression arrayExpr = children.get(0);
+
+ boolean isElementNull = false;
+
+ if (!elementExpr.evaluate(tuple, ptr)) {
+ return false;
+ } else if (ptr.getLength() == 0) {
+ isElementNull = true; // return true here immediately
+ }
{code}
- Also, in evaluate, don't create a new ptr, just use the one passed in. In
PArrayDataType.appendItemToArray, just have local variables that capture the
byte[], offset, and length of ptr before setting it to the new array.
{code}
+
+ ImmutableBytesWritable ptrElement = new ImmutableBytesPtr(ptr); // No
need for this
+
{code}
- Continuing in evaluate, this doesn't look quite right. I think you want to
only enter the if block if getMaxLength() != null and baseType.isFixedWidth().
Also, instead of returning false, you'd want to throw
DataExceedsCapacityException.
{code}
+ if (baseType.isFixedWidth())
+ {
+ int
arrayElementLength=getMaxLength()==null?baseType.getByteSize():getMaxLength();
+ if(ptrElement.getLength()>arrayElementLength)
+ {
+ return false;
+ }
+
+ }
{code}
> implement ARRAY_APPEND built in function
> ----------------------------------------
>
> Key: PHOENIX-1705
> URL: https://issues.apache.org/jira/browse/PHOENIX-1705
> Project: Phoenix
> Issue Type: Sub-task
> Reporter: Dumindu Buddhika
> Assignee: Dumindu Buddhika
> Attachments:
> PHOENIX-1705_implement_ARRAY_APPEND_built_in_function.patch,
> PHOENIX-1705_implement_ARRAY_APPEND_built_in_function.patch,
> PHOENIX-1705_implement_ARRAY_APPEND_built_in_function1.patch,
> PHOENIX-1705_implement_ARRAY_APPEND_built_in_function2.patch,
> PHOENIX-1705_implement_ARRAY_APPEND_built_in_function3.patch,
> PHOENIX-1705_implement_ARRAY_APPEND_built_in_function4.patch,
> PHOENIX-1705_implement_ARRAY_APPEND_built_in_function5.patch,
> PHOENIX-1705_implement_ARRAY_APPEND_built_in_function6.patch
>
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
