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.
---