Sitelet https://web.archive.org/web/20200710114432/https://github.com/prestodb/presto/issues/13861
Skip to content

/ presto

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

Jump to bottom

Extend array_position function with instance number #13861

Open
rongrong opened this issue Dec 12, 2019 · 11 comments
Open

Extend array_position function with instance number #13861

rongrong opened this issue Dec 12, 2019 · 11 comments
Assignees
@adaco
Labels
beginner-task syntax-needs-review

Comments

@rongrong
Copy link
Contributor

@rongrong rongrong commented Dec 12, 2019

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
Copy link
Contributor

@kaikalur kaikalur commented Dec 12, 2019

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
Copy link
Contributor

@kaikalur kaikalur commented Dec 12, 2019

BTW, this is more about function signature than syntax?
@rongrong
Copy link
Contributor Author

@rongrong rongrong commented Dec 12, 2019

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
Copy link
Contributor

@kaikalur kaikalur commented Dec 12, 2019

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
Copy link
Contributor

@kaikalur kaikalur commented Dec 12, 2019

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
Copy link

@adaco adaco commented Dec 12, 2019

I'd love to take a shot at implementing this.
@rongrong
Copy link
Contributor Author

@rongrong rongrong commented Dec 13, 2019

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
Copy link
Contributor Author

@rongrong rongrong commented Jan 10, 2020

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

@adaco adaco commented Jan 10, 2020 •
edited

@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
Copy link
Contributor Author

@rongrong rongrong commented Jan 10, 2020

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

@adaco adaco commented Jan 10, 2020

@rongrong thanks! That does the trick.
This was referenced Jan 28, 2020
Closed
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@adaco adaco

Labels
beginner-task syntax-needs-review
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
@rongrong @kaikalur @adaco
You can’t perform that action at this time.