You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@streampark.apache.org by GitBox <gi...@apache.org> on 2022/09/25 06:56:16 UTC
[GitHub] [incubator-streampark] 1996fanrui opened a new pull request, #1685: [Feature] Update the username index for t_user table
1996fanrui opened a new pull request, #1685:
URL: https://github.com/apache/incubator-streampark/pull/1685
## What problem does this PR solve?
Issue Number: close #1684
Problem Summary:
## What is changed and how it works?
Currently, the `nick_name` is the unique key for t_user. But :
- we are using username instead of nickname in many places.
- And the `nick_name` is the empty string in the `mysql-data.sql`.
- Front end doesn't allow the username is null
So I propose :
- we change the unique key from `nick_name` to `username`
- and change the `username` not null.
The old index:
> unique key `un_username` (`nick_name`) using btree
The new index:
> unique key `un_username` (`username`) using btree
Feel free to ping committers for the review!
## Contribution Checklist
- Make sure that the pull request corresponds to a [GITHUB issue](https://github.com/apache/streampark/issues).
- Name the pull request in the form "[Feature] [component] Title of the pull request", where *Feature* can be replaced by `Hotfix`, `Bug`, etc.
- Minor fixes should be named following this pattern: `[hotfix] [docs] Fix typo in README.md doc`.
## Purpose of this pull request
<!-- Describe the purpose of this pull request. For example: This pull request adds checkstyle plugin.-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] xinzhuxiansheng commented on pull request #1685: [Feature] Update the username index for t_user table
Posted by GitBox <gi...@apache.org>.
xinzhuxiansheng commented on PR #1685:
URL: https://github.com/apache/incubator-streampark/pull/1685#issuecomment-1257160572
Hi @1996fanrui , thanks for your contribution.
Can you add a pgsql-upgrad.sql to the upgrade directory to record DDL and DML changes，`pgsql-schema.sql` also needs to update the `username index`
@1996fanrui @wolfboys
upgrade directory, can we adjust it like dolphinscheduler upgrade sql?
E.g :
upgrade/${1.2.4-SNAPSHOT}_schema/${mysql/pgsql}/streampark_${ddl/dml}.sql
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] 1996fanrui commented on pull request #1685: [Feature] Update the username index for t_user table
Posted by GitBox <gi...@apache.org>.
1996fanrui commented on PR #1685:
URL: https://github.com/apache/incubator-streampark/pull/1685#issuecomment-1257186961
> > Great idea, absolutely agree. I can do it. Should we update the directory in this PR or create a new PR to do it? I prefer create a new issue and PR.
>
> @1996fanrui I agree that, we should create new issue to work.
Hi @xinzhuxiansheng @lvshaokang , I created a new issue #1687 to split `sql-upgrade.sql` into a different directory. I will do it after this PR is merged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] 1996fanrui commented on pull request #1685: [Feature] Update the username index for t_user table
Posted by GitBox <gi...@apache.org>.
1996fanrui commented on PR #1685:
URL: https://github.com/apache/incubator-streampark/pull/1685#issuecomment-1257182140
> Can you add a pgsql-upgrad.sql to the upgrade directory to record DDL and DML changes, `pgsql-schema.sql` also needs to update the `username index`
Hi @xinzhuxiansheng , thanks for your review. As I know, #1449 added the `pgsql-schema.sql` at Aug 2022. `streampark-1.2.3` doesn't support pgsql, `streampark-1.2.4` will be the first version that supports pgsql. So I think we don't need to prepare the `pgsql-upgrade.sql` at this version. Please correct me if I'm wrong.
> upgrade directory, can we adjust it like dolphinscheduler upgrade sql? E.g : upgrade/(1.2.4-SNAPSHOT)_schema/(mysql/pgsql)/streampark_(ddl/dml).sql
Great idea, absolutely agree. I can do it. Should we update the directory in this PR or create a new PR to do it? I prefer create a new issue and PR.
Actually, it is not reasonable for some feature or bugfix or upgrade to be mixed into one commit. If a commit has a bug and needs to be rolled back, multiple features will be rolled back. In the long term, we also need the `Code Style and Quality Guide` like Flink :
https://flink.apache.org/contributing/code-style-and-quality-pull-requests.html#separate-refactoring-cleanup-and-independent-changes
<img width="1105" alt="image" src="https://user-images.githubusercontent.com/38427477/192143006-61cecacd-bfab-4e52-bfc5-169a55e991ce.png">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] xinzhuxiansheng commented on pull request #1685: [Feature] Update the username index for t_user table
Posted by GitBox <gi...@apache.org>.
xinzhuxiansheng commented on PR #1685:
URL: https://github.com/apache/incubator-streampark/pull/1685#issuecomment-1257209184
> > Can you add a pgsql-upgrad.sql to the upgrade directory to record DDL and DML changes, `pgsql-schema.sql` also needs to update the `username index`
>
> Hi @xinzhuxiansheng , thanks for your review. As I know, #1449 added the `pgsql-schema.sql` at Aug 2022. `streampark-1.2.3` doesn't support pgsql, `streampark-1.2.4` will be the first version that supports pgsql. So I think we don't need to prepare the `pgsql-upgrade.sql` at this version. Please correct me if I'm wrong.
>
> > upgrade directory, can we adjust it like dolphinscheduler upgrade sql? E.g : upgrade/(1.2.4-SNAPSHOT)_schema/(mysql/pgsql)/streampark_(ddl/dml).sql
>
> Great idea, absolutely agree. I can do it. Should we update the directory in this PR or create a new PR to do it? I prefer create a new issue and PR.
>
> Actually, it is not reasonable for some feature or bugfix or upgrade to be mixed into one commit. If a commit has a bug and needs to be rolled back, multiple features will be rolled back. In the long term, we also need the `Code Style and Quality Guide` like Flink :
>
> https://flink.apache.org/contributing/code-style-and-quality-pull-requests.html#separate-refactoring-cleanup-and-independent-changes
>
> <img alt="image" width="1105" src="https://user-images.githubusercontent.com/38427477/192143006-61cecacd-bfab-4e52-bfc5-169a55e991ce.png">
@1996fanrui Thank you for your reply, i get it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys commented on pull request #1685: [Feature] Update the username index for t_user table
Posted by GitBox <gi...@apache.org>.
wolfboys commented on PR #1685:
URL: https://github.com/apache/incubator-streampark/pull/1685#issuecomment-1257197449
> > > Great idea, absolutely agree. I can do it. Should we update the directory in this PR or create a new PR to do it? I prefer create a new issue and PR.
> >
> >
> > @1996fanrui I agree that, we should create new issue to work.
>
> Hi @xinzhuxiansheng @lvshaokang , I created a new issue #1687 to split `sql-upgrade.sql` into a different directory. I will do it after this PR is merged.
very good proposal. i agree with you
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] wolfboys merged pull request #1685: [Feature] Update the username index for t_user table
Posted by GitBox <gi...@apache.org>.
wolfboys merged PR #1685:
URL: https://github.com/apache/incubator-streampark/pull/1685
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-streampark] lvshaokang commented on pull request #1685: [Feature] Update the username index for t_user table
Posted by GitBox <gi...@apache.org>.
lvshaokang commented on PR #1685:
URL: https://github.com/apache/incubator-streampark/pull/1685#issuecomment-1257184958
> Great idea, absolutely agree. I can do it. Should we update the directory in this PR or create a new PR to do it? I prefer create a new issue and PR.
@1996fanrui I agree that, we should create new issue to work.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org