Implements Display for PartitionExpr

[killme2008]

What type of enhancement is this?

User experience

What does the enhancement do?

The PartitionExpr doesn't implement Display trait currently, so the partition_expression in partitions table is not user-friendly:

(a) VALUES LESS THAN (PartitionExpr { lhs: Column("a"), op: Lt, rhs: Value(Int32(10)) })

It must be a < 10 instead.

Related code:

greptimedb/src/partition/src/expr.rs

Line 25 in c71298d

pub struct PartitionExpr {

greptimedb/src/partition/src/partition.rs

Line 71 in c71298d

impl Display for PartitionDef {

Implementation challenges

No response

[Kelvinyu1117]

Can I work on it?

[killme2008]

Can I work on it?

Of course.Thank you.

[killme2008]

@Kelvinyu1117 Just a friendly reminder, are you still working on this issue? If you require any assistance, please inform us. Thank you.

[Kelvinyu1117]

Yes, I'm working on it, just having some personal stuffs so didnt get chance to dive into the issue.

[Kelvinyu1117]

How can I test this feature locally? Do we have any unit tests that involves printing the expressions?

[killme2008]

How can I test this feature locally? Do we have any unit tests that involves printing the expressions?

Just run cargo sqlness -t partition, and you will see the different results of the test.

And of course, you must always add a unit test for the PartitionExpr and ParitionExpr, construct an instance and invoke its to_string() function to retrieve the display result, making sure it's expected.

[Kelvinyu1117]

I have run cargo sqlness -t partition, it seems no difference, but my unit test is passed. Am I missing something?
I have opened the PR, perhaps you can have a look and give some suggestions.

[killme2008]

I have run cargo sqlness -t partition, it seems no difference, but my unit test is passed. Am I missing something? I have opened the PR, perhaps you can have a look and give some suggestions.

I found the cause. Because we use the debug format of PartitionExpr here

greptimedb/src/partition/src/partition.rs

Line 66 in 88cdefa

Self::Expr(e) => write!(f, "{:?}", e),

Can you fix it too?

[killme2008]

Looks like we must change the Display impl for ParitionDef too:

greptimedb/src/partition/src/partition.rs

Line 72 in 88cdefa

fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {

Just display the partition_bounds, the columns are unnecessary currently. Am I right? @waynexia

You can run cargo sqlness -t partition to see the differences.

[Kelvinyu1117]

I have run cargo sqlness -t partition, it seems no difference, but my unit test is passed. Am I missing something? I have opened the PR, perhaps you can have a look and give some suggestions.

I found the cause. Because we use the debug format of PartitionExpr here

greptimedb/src/partition/src/partition.rs

Line 66 in 88cdefa

Self::Expr(e) => write!(f, "{:?}", e),

Can you fix it too?

Sure, let me do that.

[evenyag]

Can we close this issue now?