tidb_query: refactor utility functions of rpn_expr::impl_cast::tests module

[H-ZeX]

Signed-off-by: H-ZeX hzx20112012@gmail.com

What have you changed?

The origin ways to make_ctx and make_ret_field_type in rpn_expr::impl_cast::tests are not grace. According to @breeswish 's suggestion, I change them to other ways which are like named parameters.

What is the type of the changes?

• Engineering (engineering change which doesn't change any feature or fix any issue)

How is the PR tested?

• No code(the code this pr changed is uint test's code.)

Does this PR affect documentation (docs) or should it be mentioned in the release notes?

• No

Does this PR affect tidb-ansible?

• No

[breezewish]

How about adding structure

struct Ctx1Options {
        overflow_as_warning: bool,
        truncate_as_warning: bool,
        should_clip_to_zero: bool,
}

It is more clear.

[H-ZeX]

Yes, in rust, this way is best.

pub struct ConfigFlag {
    overflow_as_warning: bool,
    truncate_as_warning: bool,
    should_clip_to_zero: bool,
    in_insert_stmt: bool,
    in_select_stmt: bool,
}

impl Default for ConfigFlag {
    fn default() -> Self {
        ConfigFlag {
            overflow_as_warning: false,
            truncate_as_warning: false,
            should_clip_to_zero: false,
            in_insert_stmt: false,
            in_select_stmt: false,
        }
    }
}

impl From<ConfigFlag> for EvalConfig {
    fn from(c: ConfigFlag) -> Self {
        let flag = Flag::empty();
        flag |= Flag::OVERFLOW_AS_WARNING * c.overflow_as_warning as u64;
        flag |= Flag::OVERFLOW_AS_WARNING * c.truncate_as_warning as u64;
        flag |= Flag::OVERFLOW_AS_WARNING * c.should_clip_to_zero as u64;
        flag |= Flag::OVERFLOW_AS_WARNING * c.in_insert_stmt as u64;
        flag |= Flag::OVERFLOW_AS_WARNING * c.in_select_stmt as u64;
        EvalConfig { flag, max_warning_cnt: 1024 }
    }
}

#[test]
fn test_1() {
    let ctx = EvalContext {
        cfg: Arc::new(EvalConfig::from(ConfigFlag {
            overflow_as_warning: true,
            ..ConfigFlag::default()
        })),
        ..EvalContext::default()
    };
    
    // test code
}

However, if refactoring all unit tests using this way, the work is not small.

BTW, if in other language that has no ..Xx::default(), we has only two ways

• One

  #[test]
  fn test_2() {
      let mut flag: Flag = Flag::empty();
      flag |= Flag::OVERFLOW_AS_WARNING;
      let cfg = Arc::new(EvalConfig::from_flag(flag));
      let ctx = EvalContext::new(cfg);
  
      // test code
  }

• Two

  fn make_ctx_1(overflow_as_warning: bool) -> EvalContext {
      let mut flag: Flag = Flag::empty();
      flag |= Flag::OVERFLOW_AS_WARNING;
      let cfg = Arc::new(EvalConfig::from_flag(flag));
      EvalContext::new(cfg)
  }
  
  #[test]
  fn test_3() {
      let ctx = make_ctx_1(true);
      // test code
  }

The second way is better, because the first one will repeat the same code in many unit test functions, it is very easy to make mistake if repeating code.

However, how to name the make_ctx_x? If in Java, we can call all of them make_ctx. And in rust, we can simulate overload using make_ctx_1, make_ctx_2, etc., the caller need to call it according to funcName + paramList

[breezewish]

For languages that support named parameters, I think using named parameters is the best. The solution I posted here is a simulation for the named parameter.

[H-ZeX]

PTAL @lonng

[lonng]

Rest LGTM

[H-ZeX]

/tests

[andylokandy]

rtf -> rft?

[andylokandy]

Rest LGTM

[breezewish]

I'm fine with this PR. However I would like to see some utilities extract to somewhere else (in future) since they are not bounded to the cast module.

[lonng]

/merge

[sre-bot]

/run-all-tests