Skip to content

Support Custom Table Names Configuration #455

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.

Sign up for GitHub

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
Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Support Custom Table Names Configuration #455

wants to merge 6 commits into from

Conversation

@happy-game
Copy link

@happy-game happy-game commented Apr 10, 2025

  • Add a CLI parameter --table-name, with the default value being kine.
  • Add TableName to the config in pkg/endpoint.
  • Add TableName to pkg/drivers/config.go.
  • Support custom table names for sqlite, mysql, and pgsql.

I have already run scripts/test and passed all the tests.
image

#452

@happy-game happy-game requested a review from a team as a code owner April 10, 2025 12:57
brandond
brandond requested changes Apr 10, 2025
View reviewed changes
@happy-game
Copy link
Author

happy-game commented Apr 14, 2025 •
edited
Loading

Done.

@happy-game happy-game requested a review from brandond April 15, 2025 01:40
@happy-game happy-game force-pushed the master branch 2 times, most recently from 35d08b0 to 8af2ef6 Compare April 18, 2025 15:21
@happy-game
Copy link
Author

happy-game commented Apr 21, 2025

Hi, @brandond , I have resolved the problems mentioned previously. Could you please review this pull request at your convenience? If there is anything that needs improvement on this PR, I will deal with it as soon as possible.

brandond
brandond requested changes Apr 22, 2025
View reviewed changes
Copy link
Member

@brandond brandond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couple more nits

@happy-game happy-game requested a review from brandond April 23, 2025 12:38
@happy-game
Copy link
Author

happy-game commented Apr 24, 2025

Apologies, I amended a small change (adjusting CLI parameter position) onto a previous commit. This changed the subsequent commit hashes. I'm very sorry that this increased your review workload.

@happy-game happy-game mentioned this pull request Apr 28, 2025
Open
@happy-game
Copy link
Author

happy-game commented Apr 28, 2025

Hi @brandond ,I've fixed the issue you mentioned previously. Could you please review the code at your convenience?
Sorry for the extra work caused by my previous mistake.

@brandond
Copy link
Member

brandond commented May 9, 2025 •
edited
Loading

This looks good but I'd like a chance to look it over a bit more closely to think if there's some simpler way to do it without adding so much string mangling and new functions. I'll try to get to that next week.

It also needs a rebase, sorry!

happy-game added 6 commits May 12, 2025 15:25
@happy-game
feat: add a custom table name in the config
e004b39 
Signed-off-by: happy game <[email protected]>
@happy-game
feat: provide support for custom table names in SQLite
38300c3 
- Modify the SQL statements in `pkg/drivers/generic` to adapt to custom table names.
- Modify the SQL statements in `pkg/drivers/sqlite` to adapt to custom table names.

Signed-off-by: happy game <[email protected]>
@happy-game
feat: provide support for custom table name
f00544c 
- support mysql
- support pgsql

Signed-off-by: happy game <[email protected]>
@happy-game
refactor(generic): Modify SQL statements generation
93818f8 
- Move SQL statement generation from struct methods to package-level variables
- Validate table name
- Quote table name
- Reverts the pattern of setting fields after variable declaration

Signed-off-by: happy game <[email protected]>
@happy-game
refactor(drivers): Modify the concatenation method of SQL statements
7065afe 
- Use + to concatenate SQL statements instead of fmt.Sprintf

Signed-off-by: happy game <[email protected]>
@happy-game
refactor(drivers): Unify table name quoting
d52aedf 
- Quote table names using double quotes (").
- For MySQL, add `ANSI_QUOTES` to the `sql_mode` parameter in the DSN.

Signed-off-by: happy game <[email protected]>
@happy-game
Copy link
Author

happy-game commented May 12, 2025

I have rebased onto the latest master branch.

@happy-game
Copy link
Author

happy-game commented Jul 2, 2025

Hi @brandond，it has been more than a month since this PR was submitted. May I inquire whether this PR can be merged or if there are still areas that need modification? Thanks.

@brandond
Copy link
Member

brandond commented Sep 16, 2025 •
edited
Loading

  1. This needs rebasing again, sorry
  2. I know our SQL generation is already a mess, but honestly I think we need to refactor it a bit more before we do anything like this - it makes what is already a confusing mess of SQL snippets even more obtuse. Can you think of a good way to do this that's perhaps a bit cleaner?

@happy-game
Copy link
Author

happy-game commented Sep 17, 2025

If you still feel this feature is necessary, I'm happy to implement it. However, I would prefer to start fresh on the latest commit and open a new pull request.
On the one hand, I haven't looked at this PR in a while, and it's a bit of a mess. On the other hand, writing the code from scratch would likely be easier than trying to rebase and resolve all the conflicts.
I'll give it more thought this time to ensure the new PR is much cleaner and more focused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brandond brandond Awaiting requested review from brandond

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@happy-game @brandond