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

add custom field name support (NameFn) #50

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ashtonian
Copy link
Contributor

I added nameFn function the same way we handle column names and defaulted the behavior to match the current behavior so its not breaking, added tests to verify. This is a reopening of #47. PR is now independent of other prs. Added test coverage, good to go on approval.

Original #18.

@sashahilton00
Copy link

Just as a heads up @ashtonian the merge commit on your main repo (#d5fc95e) appears to have broken the NameFn logic that was added - I'd open a PR but having scrolled back through the commits there's been quite a bit of restructuring from the main repo, and I was unable to figure things out to open one.

If you get a chance to take a look it would be much appreciated - I'm hoping to take a stab at adding the table= option for fields to allow querying across tables, though I'd like to start from a clean, up to date version first :)

❯ go test
--- FAIL: TestParse (0.00s)
    --- FAIL: TestParse/support_name_struct_opt (0.00s)
        rql_test.go:1121: filter expr:
            	got: "someName = ?"
            	want "some_name = ?"
    --- FAIL: TestParse/test_nameFn_works (0.00s)
        rql_test.go:1121: filter expr:
            	got: "someName = ?"
            	want "some_name = ?"
FAIL
exit status 1
FAIL	github.com/ashtonian/rql	0.144s

@ashtonian
Copy link
Contributor Author

Hey @sashahilton00, unfortunately I'm not sure when I will be able to prioritize this. I have some things I'd like to do with this project but currently on a 3 month timeline. May be worthwhile to ignore that branch. My intent was to do some refactors to make the project more flexible but I think the strategy I initially chose is not quite right and there is a more robust/functional approach.

@sashahilton00
Copy link

Hi @ashtonian,

Thanks for the heads up. I started looking into how one might handle the nested tables approach, and also concluded that it needed quite a significant refactor due to the way the internal structs are initialised/processed.

In case it's of interest to you or anyone else looking to solve a problem similar to mine - my quick and dirty solution is to run a TypeSense instance, with a thin DSL on top of the search/query filters they provide.

Probably a bit heavy for most use cases, but I was using it already.

Nonetheless I'll keep an eye out on this library and your progress - I'm surprised there isn't a more fully fleshed out solution to this requirement already available to be honest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants