Presto: Extend array_position function with instance number

[rongrong]

Currently array_position only returns the first occurrence of the given element. We want to extend array_position to take an additional parameter instance similar to strpos.

array_position(x, element, instance) -> bigint

For example, for array x: [1, 3, 2, 1, 4, 3, 2, 5, 4, 1]:

array_position(x, 1) = 1 -- existing function
array_position(x, 1, 1) = 1 -- same as existing function with 2 input parameters
array_position(x, 1, 2) = 4 -- returns index of the second occurrence of 1
array_position(x, 5, 2) = 0 -- returns 0 because there is no second occurrence of 5
array_position(x, 1, -1) = 10 -- returns index of the first occurrence of 1 from the back
array_posision(x, 1, -2) = 4 -- returns index of the second occurrence of 1 from the back

[kaikalur]

I wonder if offset is better than instance - especially when going backwards. But I see we already have strpos and strrpos with instance as a param so I'm not sure if we want a different style for arrays.

[kaikalur]

BTW, this is more about function signature than syntax?

[rongrong]

offset sounds more like the return value (index) to me. But I picked instance just because that's what it's called in strpos. Don't feel any strong motivation to change it but I'm open to ideas.

[kaikalur]

offset sounds more like the return value (index) to me. But I picked instance just because that's what it's called in strpos. Don't feel any strong motivation to change it but I'm open to ideas.

Only concern is the negative instance. strrpos did not have that problem. May be we do array_reverse_position instead and be consistent with the strrpos?

[kaikalur]

offset sounds more like the return value (index) to me. But I picked instance just because that's what it's called in strpos. Don't feel any strong motivation to change it but I'm open to ideas.

Only concern is the negative instance. strrpos did not have that problem. May be we do array_reverse_position instead and be consistent with the strrpos?

Or array_rposition?

[adaco]

I'd love to take a shot at implementing this.

[rongrong]

offset sounds more like the return value (index) to me. But I picked instance just because that's what it's called in strpos. Don't feel any strong motivation to change it but I'm open to ideas.

Only concern is the negative instance. strrpos did not have that problem. May be we do array_reverse_position instead and be consistent with the strrpos?

You mean that negative instance doesn't make sense?

Between array_reverse_position and array_rposition I prefer the former. Though I still like the negative number idea better. So it's nicer if we can get a better name.

[rongrong]

@achao Are you still planning to work on this? Any updates?

[adaco]

@rongrong yup! Struggling a bit ATM to build the project (following the steps here).

./mvnw -e clean install
...
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.0:compile (default-compile) on project presto-cache: Compilation failure: Compilation failure: 
[ERROR] ~/presto/presto-cache/src/main/java/com/facebook/presto/cache/LocalRangeCacheManager.java:[30,24] cannot find symbol
[ERROR]   symbol:   class PreDestroy
[ERROR]   location: package javax.annotation
[ERROR] ~/presto/presto-cache/src/main/java/com/facebook/presto/cache/LocalRangeCacheManager.java:[134,6] cannot find symbol
[ERROR]   symbol:   class PreDestroy
[ERROR]   location: class com.facebook.presto.cache.LocalRangeCacheManager
[ERROR] -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.0:compile (default-compile) on project presto-cache: Compilation failure
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:213)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:154)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:146)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:117)
    at org.apache.maven.lifecycle.internal.builder.multithreaded.MultiThreadedBuilder$1.call(MultiThreadedBuilder.java:200)
    at org.apache.maven.lifecycle.internal.builder.multithreaded.MultiThreadedBuilder$1.call(MultiThreadedBuilder.java:196)
    at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
    at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
    at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
    at java.base/java.lang.Thread.run(Thread.java:830)
Caused by: org.apache.maven.plugin.compiler.CompilationFailureException: Compilation failure
    at org.apache.maven.plugin.compiler.AbstractCompilerMojo.execute(AbstractCompilerMojo.java:1215)
    at org.apache.maven.plugin.compiler.CompilerMojo.execute(CompilerMojo.java:188)
    at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:134)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:208)
    ... 11 more

Any ideas?

$ java --version
java 13.0.1 2019-10-15
Java(TM) SE Runtime Environment (build 13.0.1+9)
Java HotSpot(TM) 64-Bit Server VM (build 13.0.1+9, mixed mode, sharing)

$ mvn --version
Apache Maven 3.5.0 (ff8f5e7444045639af65f6095c62210b5713f426; 2017-04-03T15:39:06-04:00)
Maven home: /opt/homebrew/Cellar/maven/3.5.0/libexec
Java version: 13.0.1, vendor: Oracle Corporation
Java home: /Library/Java/JavaVirtualMachines/jdk-13.0.1.jdk/Contents/Home
Default locale: en_US, platform encoding: UTF-8
OS name: "mac os x", version: "10.14.6", arch: "x86_64", family: "mac"

[rongrong]

I use java 1.8. Try export JAVA_HOME=$(/usr/libexec/java_home -v 1.8) and see whether it works.

[adaco]

@rongrong thanks! That does the trick.

[ssaumitra]

@adaco Are you still working on this?

[ssaumitra]

@adaco I am assigning this to myself.