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.

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

Implement the POSITION function #8969

Closed
alamb opened this issue Jan 23, 2024 · 7 comments
Closed

Implement the POSITION function #8969

alamb opened this issue Jan 23, 2024 · 7 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@alamb
Copy link
Contributor

alamb commented Jan 23, 2024

Is your feature request related to a problem or challenge?

The idea is to support the position function from postgres:

https://www.postgresql.org/docs/9.1/functions-string.html

position(substring in string)	

For example

postgres=# select position('world' in 'Helloworld');
 position
----------
        6
(1 row)

Describe the solution you'd like

As @zy-kkk points out on #8862 (comment) postgres has special syntax

To support the syntax position('world' in 'Helloworld') = 6 we probably need to do it in at https://github.com/sqlparser-rs/sqlparser-rs and add support for this syntax , which may be like adding the overlay function in apache/datafusion-sqlparser-rs#594.

Then we can change the datafusion planner to recognize that special syntax and call the instr function in #8862

Describe alternatives you've considered

No response

Additional context

This came up while implementing #8862

@alamb alamb added the enhancement New feature or request label Jan 23, 2024
@alamb alamb added the good first issue Good for newcomers label Jan 23, 2024
@alamb
Copy link
Contributor Author

alamb commented Jan 23, 2024

I think this is a good first issue, though it will require updating sqlparser-rs and then waiting for that to be released and making a change in datafusion

@Lordworms
Copy link
Contributor

I would like to try that

Lordworms added a commit to Lordworms/arrow-datafusion that referenced this issue Jan 24, 2024
@Lordworms
Copy link
Contributor

Hello, @alamb I have created a PR plz review it if you have time. Additionally, I have some other problems:

  1. what is the suitable resolution time for a picked-up issue?
  2. It might be slow and other problems may happen if I test the full test locally, may I create a PR first to test it?
  3. the rust-analyzer really consumes my CPU and RAM, is there any better way for a better Rust environment?
    I would appreciate so much for your answer

@alamb
Copy link
Contributor Author

alamb commented Jan 25, 2024

Hello, @alamb I have created a PR plz review it if you have time. Additionally, I have some other problems:

Thank you @Lordworms - I have it on my

  1. what is the suitable resolution time for a picked-up issue?

From my perspective there is no particular deadline for any issue.

Instead, if someone is working on an issue but wasn't able to finish it before someone else needs / wants to work on the same thing, that other person can simply make a PR.

I think most contributors are polite and check with the person who first signaled interest before working on something, but just because someone else might be working on something doesn't prevent anyone else from doing so

  1. It might be slow and other problems may happen if I test the full test locally, may I create a PR first to test it?

I think making a draft PR to run the full suite of tests so you know which ones may need to run locally is a very reasonable strategy

  1. the rust-analyzer really consumes my CPU and RAM, is there any better way for a better Rust environment?
    I would appreciate so much for your answer

I don't have a good answer for this -- Rust development environments in general do require significant resources

@Lordworms
Copy link
Contributor

Thank you, got it!

Lordworms added a commit to Lordworms/arrow-datafusion that referenced this issue Jan 25, 2024
Lordworms added a commit to Lordworms/arrow-datafusion that referenced this issue Jan 25, 2024
Lordworms added a commit to Lordworms/arrow-datafusion that referenced this issue Jan 25, 2024
Lordworms added a commit to Lordworms/arrow-datafusion that referenced this issue Jan 25, 2024
Lordworms added a commit to Lordworms/arrow-datafusion that referenced this issue Jan 25, 2024
Lordworms added a commit to Lordworms/arrow-datafusion that referenced this issue Jan 25, 2024
Lordworms added a commit to Lordworms/arrow-datafusion that referenced this issue Jan 25, 2024
Lordworms added a commit to Lordworms/arrow-datafusion that referenced this issue Jan 26, 2024
alamb pushed a commit that referenced this issue Feb 2, 2024
* feat: issue #8969 adding position function

* reuse Instr
@SteveLauC
Copy link
Contributor

Seems that we should close this issue as it was completed in #8988

@alamb
Copy link
Contributor Author

alamb commented Feb 29, 2024

Seems that we should close this issue as it was completed in #8988

Thank you @SteveLauC -- good call

@alamb alamb closed this as completed Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants