gluesql icon indicating copy to clipboard operation
gluesql copied to clipboard

Merge `BlendContext` and `FilterContext` into a single `Context` struct

Open panarch opened this issue 3 years ago • 0 comments

In core/src/executor/context/, there exists two contexts which have slightly different roles - BlendContext and FilterContext.

  • FilterContext
#[derive(Debug)]
enum Content<'a> {
    Some {
        table_alias: &'a str,
        columns: Rc<[String]>,
        row: Option<&'a Row>,
    },
    None,
}

#[derive(Debug)]
pub struct FilterContext<'a> {
    content: Content<'a>,
    next: Option<Rc<FilterContext<'a>>>,
    next2: Option<Rc<BlendContext<'a>>>,
}
  • BlendContext
#[derive(Debug)]
pub struct BlendContext<'a> {
    table_alias: &'a str,
    columns: Rc<[String]>,
    row: Option<Row>,
    next: Option<Rc<BlendContext<'a>>>,
}

BlendContext and FilterContext now have a very similar look and they also even shares internal method behaviors. e.g.

impl<'a> FilterContext<'a> {
    pub fn new(..) { .. }
    pub fn get_value(&'a self, target: &str) -> Option<&'a Value>;
    pub fn get_alias_value(&'a self, target_alias: &str, target: &str) -> Option<&'a Value>;
}

impl<'a> BlendContext<'a> {
    pub fn new(..) { .. }
    pub fn get_value(&'a self, target: &str) -> Option<&'a Value>;
    pub fn get_alias_value(&'a self, table_alias: &str, target: &str) -> Option<&'a Value>;
    pub fn get_alias_values(&self, alias: &str) -> Option<Vec<Value>>;
    pub fn get_all_values(&'a self) -> Vec<Value>;

There were several trials to merge these into a single context module. Though these trials were not fully successful, there were some progress by making the looks of both contexts have more similar shapes. Now.. we may be possible to finally merge both contexts into a single one.

panarch avatar Aug 15 '22 11:08 panarch