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

[FixBug] Fix lose of meta data bug after alter routine load #6937

Merged
merged 4 commits into from
Jun 7, 2022

Conversation

gengjun-git
Copy link
Contributor

@gengjun-git gengjun-git commented Jun 6, 2022

What type of PR is this:

  • bug
  • feature
  • enhancement
  • refactor
  • others

Which issues of this PR fixes :

Fixes #6936

Problem Summary(Required) :

When routine load serializes the load properties(columns/rows separator, column list, partitions, where filter), it stores the sql statement, and get the load properties by parsing the sql for deserialization. But after alter routine load is done, it will only keep the alter statement, this will cause the loss of metadata. We should merge the alter sql with the origin create sql to retain all load properties.

}
} catch (Exception e) {
LOG.error("error happens when parsing create/alter routine load stmt: " + origStmt.originStmt, e);
return null;
Copy link
Contributor

Choose a reason for hiding this comment

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

why return null not throw exception, inconsistent with previous behavior

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If exception is thrown here, the fe will start failed. I'm afraid that there may be some corner case I have not considered. This is not a critical problem, so we just return null, and the load desc can be null.

padmejin
padmejin previously approved these changes Jun 7, 2022
Copy link
Contributor

@padmejin padmejin left a comment

Choose a reason for hiding this comment

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

LGTM

Astralidea
Astralidea previously approved these changes Jun 7, 2022
@Astralidea
Copy link
Contributor

run starrocks_fe_unittest

@gengjun-git gengjun-git dismissed stale reviews from Astralidea and padmejin via 3e6d0ff June 7, 2022 07:41
@wanpengfei-git
Copy link
Collaborator

[FE PR Coverage check]

😍 pass : 80 / 85 (94.12%)

file detail

path covered line new line coverage
🔵 com/starrocks/load/routineload/RoutineLoadManager.java 0 2 00.00%
🔵 com/starrocks/load/routineload/RoutineLoadJob.java 27 29 93.10%
🔵 com/starrocks/load/RoutineLoadDesc.java 37 38 97.37%
🔵 com/starrocks/analysis/CreateRoutineLoadStmt.java 16 16 100.00%

@gengjun-git gengjun-git merged commit aacea9d into StarRocks:main Jun 7, 2022
gengjun-git added a commit to gengjun-git/starrocks that referenced this pull request Jun 7, 2022
…s#6937)

When routine load serializes the load properties(columns/rows separator, column list, partitions, where filter), it stores the sql statement, and get the load properties by parsing the sql for deserialization. But after alter routine load is done, it will only keep the alter statement, this will cause the loss of metadata. We should merge the alter sql with the origin create sql to retain all load properties.
gengjun-git added a commit that referenced this pull request Jun 7, 2022
…6971)

When routine load serializes the load properties(columns/rows separator, column list, partitions, where filter), it stores the sql statement, and get the load properties by parsing the sql for deserialization. But after alter routine load is done, it will only keep the alter statement, this will cause the loss of metadata. We should merge the alter sql with the origin create sql to retain all load properties.
gengjun-git added a commit to gengjun-git/starrocks that referenced this pull request Jun 10, 2022
…s#6937) (StarRocks#6971)

When routine load serializes the load properties(columns/rows separator, column list, partitions, where filter), it stores the sql statement, and get the load properties by parsing the sql for deserialization. But after alter routine load is done, it will only keep the alter statement, this will cause the loss of metadata. We should merge the alter sql with the origin create sql to retain all load properties.
gengjun-git added a commit to gengjun-git/starrocks that referenced this pull request Jun 10, 2022
…s#6937)

When routine load serializes the load properties(columns/rows separator, column list, partitions, where filter), it stores the sql statement, and get the load properties by parsing the sql for deserialization. But after alter routine load is done, it will only keep the alter statement, this will cause the loss of metadata. We should merge the alter sql with the origin create sql to retain all load properties.
gengjun-git added a commit that referenced this pull request Jun 10, 2022
…6971) (#7068)

When routine load serializes the load properties(columns/rows separator, column list, partitions, where filter), it stores the sql statement, and get the load properties by parsing the sql for deserialization. But after alter routine load is done, it will only keep the alter statement, this will cause the loss of metadata. We should merge the alter sql with the origin create sql to retain all load properties.
chaoyli pushed a commit that referenced this pull request Jun 11, 2022
When routine load serializes the load properties(columns/rows separator, column list, partitions, where filter), it stores the sql statement, and get the load properties by parsing the sql for deserialization. But after alter routine load is done, it will only keep the alter statement, this will cause the loss of metadata. We should merge the alter sql with the origin create sql to retain all load properties.
abc982627271 pushed a commit to abc982627271/starrocks that referenced this pull request Jun 22, 2022
…s#6937)

When routine load serializes the load properties(columns/rows separator, column list, partitions, where filter), it stores the sql statement, and get the load properties by parsing the sql for deserialization. But after alter routine load is done, it will only keep the alter statement, this will cause the loss of metadata. We should merge the alter sql with the origin create sql to retain all load properties.
jaogoy pushed a commit to jaogoy/starrocks that referenced this pull request Nov 15, 2023
* fix linting

Signed-off-by: DanRoscigno <dan@roscigno.com>

* fix linting

Signed-off-by: DanRoscigno <dan@roscigno.com>

* fix linting

Signed-off-by: DanRoscigno <dan@roscigno.com>

* fix links in 2.3

---------

Signed-off-by: DanRoscigno <dan@roscigno.com>
Co-authored-by: evelyn.zhaojie <98087056+evelynzhaojie@users.noreply.github.com>
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.

[BUG] Some load parameters are lost after alter routine load
6 participants