Github user iyerr3 commented on a diff in the pull request:

    https://github.com/apache/madlib/pull/316#discussion_r217260530
  
    --- Diff: src/ports/postgres/modules/utilities/control.py_in ---
    @@ -158,6 +159,61 @@ class MinWarning(ContextDecorator):
                              format(oldMsgLevel=self.oldMsgLevel))
     
     
    +class AOControl(ContextDecorator):
    +
    +    """
    +    @brief: A wrapper that enables/disables the AO storage option
    +    """
    +
    +    def __init__(self, enable=False):
    +        self.to_enable = enable
    +        self.was_ao_enabled = False
    +        self.guc_exists = True
    +        self.storage_options_dict = dict()
    +
    +    def _parse_gp_default_storage_options(self, 
gp_default_storage_options_str):
    +        """ Parse comma separated key=value pairs
    +
    +        Example:
    +             
appendonly=false,blocksize=32768,compresstype=none,checksum=true,orientation=row
    +        """
    +        self.storage_options_dict = 
extract_keyvalue_params(gp_default_storage_options_str)
    +        self.storage_options_dict['appendonly'] = bool(
    +            strtobool(self.storage_options_dict['appendonly']))
    +
    +    def _join_gp_defaut_storage_options(self):
    +        return ','.join(['{0}={1}'.format(k, v)
    +                        for k, v in self.storage_options_dict.iteritems()])
    +
    +    def __enter__(self):
    +        try:
    +            _storage_options_str = plpy.execute(
    +                "show 
gp_default_storage_options")[0]["gp_default_storage_options"]
    +            self._parse_gp_default_storage_options(_storage_options_str)
    +
    +            # Set APPENDONLY=False after backing up existing value
    +            self.was_ao_enabled = self.storage_options_dict['appendonly']
    +            self.storage_options_dict['appendonly'] = self.to_enable
    +            plpy.execute("set gp_default_storage_options={0}".
    +                         format(self._join_gp_defaut_storage_options()))
    +        except plpy.SPIError:
    +            self.guc_exists = False
    +        finally:
    +            return self
    +
    +    def __exit__(self, *args):
    +        if args and args[0]:
    +            # an exception was raised in code. We return False so that any
    +            # exception is re-raised after exit. The transaction will not
    +            # commit leading to reset of parameter value.
    --- End diff --
    
    The GUC value should also reset if transaction does not commit. Ideally, 
this context manager should be called at the highest level. However, as you 
mention, it's possible it's not used correctly, with the raised exception 
caught and ignored. I have updated the code to perform the reset before 
re-raising the exception. 


---

Reply via email to