Thread

Commits

  1. Grammar fixes for split/merge partitions code

  1. Revise some error messages in split partition code

    Richard Guo <guofenglinux@gmail.com> — 2024-04-10T11:32:21Z

    I noticed some error messages in the split partition code that are not
    up to par.  Such as:
    
    "new partitions not have value %s but split partition has"
    
    how about we revise it to:
    
    "new partitions do not have value %s but split partition does"
    
    Another one is:
    
    "any partition in the list should be DEFAULT because split partition is
    DEFAULT"
    
    how about we revise it to:
    
    "all partitions in the list should be DEFAULT because split partition is
    DEFAULT"
    
    Another problem I noticed is that in the test files partition_split.sql
    and partition_merge.sql, there are comments specifying the expected
    error messages for certain test queries.  However, in some cases, the
    error message mentioned in the comment does not match the error message
    actually generated by the query.  Such as:
    
    -- ERROR:  invalid partitions order, partition "sales_mar2022" can not be
    merged
    -- (space between sections sales_jan2022 and sales_mar2022)
    ALTER TABLE sales_range MERGE PARTITIONS (sales_jan2022, sales_mar2022)
    INTO sales_jan_mar2022;
    ERROR:  lower bound of partition "sales_mar2022" conflicts with upper bound
    of previous partition "sales_jan2022"
    
    I'm not sure if it's a good practice to specify the expected error
    message in the comment.  But if we choose to do so, I think we at least
    need to ensure that the specified error message in the comment remains
    consistent with the error message produced by the query.
    
    Also there are some comments containing grammatical issues.  Such as:
    
    -- no error: bounds of sales_noerror equals to lower and upper bounds of
    sales_dec2022 and sales_feb2022
    
    Attached is a patch to fix the issues I've observed.  I suspect there
    may be more to be found.
    
    Thanks
    Richard
    
  2. Re: Revise some error messages in split partition code

    Tender Wang <tndrwang@gmail.com> — 2024-04-10T13:32:32Z

    Richard Guo <guofenglinux@gmail.com> 于2024年4月10日周三 19:32写道:
    
    > I noticed some error messages in the split partition code that are not
    > up to par.  Such as:
    >
    > "new partitions not have value %s but split partition has"
    >
    > how about we revise it to:
    >
    > "new partitions do not have value %s but split partition does"
    >
    > Another one is:
    >
    > "any partition in the list should be DEFAULT because split partition is
    > DEFAULT"
    >
    > how about we revise it to:
    >
    > "all partitions in the list should be DEFAULT because split partition is
    > DEFAULT"
    >
    > Another problem I noticed is that in the test files partition_split.sql
    > and partition_merge.sql, there are comments specifying the expected
    > error messages for certain test queries.  However, in some cases, the
    > error message mentioned in the comment does not match the error message
    > actually generated by the query.  Such as:
    >
    > -- ERROR:  invalid partitions order, partition "sales_mar2022" can not be
    > merged
    > -- (space between sections sales_jan2022 and sales_mar2022)
    > ALTER TABLE sales_range MERGE PARTITIONS (sales_jan2022, sales_mar2022)
    > INTO sales_jan_mar2022;
    > ERROR:  lower bound of partition "sales_mar2022" conflicts with upper
    > bound of previous partition "sales_jan2022"
    >
    > I'm not sure if it's a good practice to specify the expected error
    > message in the comment.  But if we choose to do so, I think we at least
    > need to ensure that the specified error message in the comment remains
    > consistent with the error message produced by the query.
    >
    > Also there are some comments containing grammatical issues.  Such as:
    >
    > -- no error: bounds of sales_noerror equals to lower and upper bounds of
    > sales_dec2022 and sales_feb2022
    >
    > Attached is a patch to fix the issues I've observed.  I suspect there
    > may be more to be found.
    >
    
    Yeah. The day before yesterday I found some grammer errors from error
    messages and code comments [1] .
    Except  those issues, @Alexander Lakhin <exclusion@gmail.com>  has found
    some bugs [2]
    I have some concerns that whether this patch is ready to commit.
    
    
    [1]
    https://www.postgresql.org/message-id/CAHewXNkGMPU50QG7V6Q60JGFORfo8LfYO1_GCkCa0VWbmB-fEw%40mail.gmail.com
    [2]
    https://www.postgresql.org/message-id/dbc8b96c-3cf0-d1ee-860d-0e491da20485%40gmail.com
    
    -- 
    Tender Wang
    OpenPie:  https://en.openpie.com/