ArulJerald commented on issue #3977: URL: https://github.com/apache/iceberg-python/issues/3977#issuecomment-5707341947
I'd like to pick it up and raise the PR. @sungwy - Could you please clarify ? <p style="white-space: pre-wrap; margin-top: 0.1em; margin-bottom: 0.2em; unicode-bidi: plaintext; color: rgb(191, 191, 191); font-family: -apple-system, "system-ui", "Segoe UI", Roboto, sans-serif; font-size: 13px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(18, 19, 20); text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;">While working on this I hit a design question worth settling before opening a PR.</p><p style="white-space: pre-wrap; margin-top: 0.1em; margin-bottom: 0.2em; unicode-bidi: plaintext; color: rgb(191, 191, 191); font-family: -apple-system, "system-ui", "Segoe UI", Roboto, sans-serif; font-size: 13px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(18, 19, 20); text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;">The straightforward fix is to rebuild <code style="font-family: monospace; color: rgb(140, 140, 140); background: none 0% 0% / auto repeat scroll padding-box border-box rgb(36, 37, 38); padding: 2px 4px; border-radius: 3px; word-break: break-word; font-size: 0.9em;">Parameters</code> from the reserved keys (<code style="font-family: monospace; color: rgb(140, 140, 140); background: none 0% 0% / auto repeat scroll padding-box border-box rgb(36, 37, 38); padding: 2px 4px; border-radius: 3px; word-break: break-word; font-size: 0.9em;">table_type</code>, <code style="font-family: monospace; color: rgb(140, 140, 140 ); background: none 0% 0% / auto repeat scroll padding-box border-box rgb(36, 37, 38); padding: 2px 4px; border-radius: 3px; word-break: break-word; font-size: 0.9em;">metadata_location</code>, <code style="font-family: monospace; color: rgb(140, 140, 140); background: none 0% 0% / auto repeat scroll padding-box border-box rgb(36, 37, 38); padding: 2px 4px; border-radius: 3px; word-break: break-word; font-size: 0.9em;">previous_metadata_location</code>) plus the current table properties, rather than layering onto whatever Glue already holds. That prunes removed properties correctly.</p><p style="white-space: pre-wrap; margin-top: 0.1em; margin-bottom: 0.2em; unicode-bidi: plaintext; color: rgb(191, 191, 191); font-family: -apple-system, "system-ui", "Segoe UI", Roboto, sans-serif; font-size: 13px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px ; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(18, 19, 20); text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;">It also drops keys PyIceberg never wrote, though. Injecting externally-managed parameters into a moto-backed Glue table and then running an ordinary property commit:</p> key | today | with rebuild -- | -- | -- classification | preserved | deleted UPDATED_BY_CRAWLER | preserved | deleted recordCount | preserved | deleted <p style="white-space: pre-wrap; margin-top: 0.1em; margin-bottom: 0.2em; unicode-bidi: plaintext; color: rgb(191, 191, 191); font-family: -apple-system, "system-ui", "Segoe UI", Roboto, sans-serif; font-size: 13px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(18, 19, 20); text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;">So parameters written by Athena or a Glue crawler would disappear on the next commit.</p><p style="white-space: pre-wrap; margin-top: 0.1em; margin-bottom: 0.2em; unicode-bidi: plaintext; color: rgb(191, 191, 191); font-family: -apple-system, "system-ui", "Segoe UI", Roboto, sans-serif; font-size: 13px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(18, 19, 20); text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;">The alternative is to prune surgically — remove only keys that were in the previous table properties and are absent from the new ones, leaving anything else alone. Both <code style="font-family: monospace; color: rgb(140, 140, 140); background: none 0% 0% / auto repeat scroll padding-box border-box rgb(36, 37, 38); padding: 2px 4px; border-radius: 3px; word-break: break-word; font-size: 0.9em;">current_glue_table</code> and <code style="font-family: monospace; color: rgb(140, 140, 140); background: none 0% 0% / auto repeat scroll padding-box border-box rgb(36, 37, 38); padding: 2px 4px; border-radius: 3px; word-break: break-word; font-size: 0.9em;">current_table.properties</code> are already in scope at the <code style="font-family: monospace; color: rgb(140, 140, 140); background: none 0% 0% / auto repeat scroll padding-box border-box rgb(36, 37, 38); padding: 2px 4px; border-radius: 3px; word-break: break-word; font-size: 0.9em;">commit_table</code> call site, so it stays small:</p><div class="codeBlockWrapper_-a7MRw" style="position: relative; margin: 8px 0px; color: rgb(191, 191, 191); font-family: -apple-system, "system-ui", "Segoe UI", Roboto, sans-serif; font-size: 13px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: normal; background-color: rgb(18, 19, 20); text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;"><button class="cop yButton_CEmTFw copyButton_-a7MRw" title="Copy code" aria-label="Copy code to clipboard" style="color: rgb(191, 191, 191); font-family: -apple-system, "system-ui", "Segoe UI", Roboto, sans-serif; font-size: 13px; background: none 0% 0% / auto repeat scroll padding-box border-box rgb(18, 19, 20); border-color: rgb(42, 43, 44); border-style: solid; border-width: 1px; border-image: none 100% / 1 / 0 stretch; cursor: pointer; opacity: 0; display: flex; border-radius: 4px; justify-content: center; align-items: center; padding: 4px; transition: opacity 0.15s, background 0.15s; position: absolute; top: 4px; right: 4px;"><svg xmlns="http://www.w3.org/2000/svg"; viewBox="0 0 20 20" fill="currentColor" aria-hidden="true" data-slot="icon" class="copyIcon_CEmTFw"><path fill-rule="evenodd" d="M15.988 3.012A2.25 2.25 0 0 1 18 5.25v6.5A2.25 2.25 0 0 1 15.75 14H13.5v-3.379a3 3 0 0 0-.879-2.121l-3.12-3.121a3 3 0 0 0-1.402-.791 2.252 2.252 0 0 1 1.913-1.576A2.25 2.25 0 0 1 12.25 1h1 .5a2.25 2.25 0 0 1 2.238 2.012ZM11.5 3.25a.75.75 0 0 1 .75-.75h1.5a.75.75 0 0 1 .75.75v.25h-3v-.25Z" clip-rule="evenodd"></path><path d="M3.5 6A1.5 1.5 0 0 0 2 7.5v9A1.5 1.5 0 0 0 3.5 18h7a1.5 1.5 0 0 0 1.5-1.5v-5.879a1.5 1.5 0 0 0-.44-1.06L8.44 6.439A1.5 1.5 0 0 0 7.378 6H3.5Z"></path></svg></button><pre style="overflow-x: auto; white-space: pre; background: none 0% 0% / auto repeat scroll padding-box border-box rgb(36, 37, 38); box-sizing: border-box; border-radius: 4px; max-width: 100%; margin: 0px; padding: 8px;"><code class="language-python" style="font-family: monospace; color: rgb(140, 140, 140); background: 0px 0px; padding: 0px; border-radius: 3px; word-break: break-word; font-size: 0.9em;">new_parameters = dict(glue_table.get("Parameters", {})) if glue_table else {} for key in set(prev_metadata_properties) - set(metadata_properties or {}): new_parameters.pop(key, None) </code></pre></div><p style="white-space: pre-wrap; margin-top: 0.1em; margin-bottom: 0.2em; unicode-bidi: plaintext; color: rgb(191, 191, 191); font-family: -apple-system, "system-ui", "Segoe UI", Roboto, sans-serif; font-size: 13px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(18, 19, 20); text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;">Which behavior do you want?</p><ol style="padding-inline-start: 2em; color: rgb(191, 191, 191); font-family: -apple-system, "system-ui", "Segoe UI", Roboto, sans-serif; font-size: 13px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: normal; background-color: rgb(18, 19, 20); text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;"><li style="unicode-bidi: plaintext;"><strong>Full rebuild</strong><span> </span>— Glue<span> </span><code style="font-family: monospace; color: rgb(140, 140, 140); background: none 0% 0% / auto repeat scroll padding-box border-box rgb(36, 37, 38); padding: 2px 4px; border-radius: 3px; word-break: break-word; font-size: 0.9em;">Parameters</code><span> </span>becomes an exact mirror of the Iceberg table properties; anything else isn't PyIceberg's to keep.</li><li style="unicode-bidi: plaintext;"><strong>Surgical prune</strong><span> </span>— only properties the table itself dropped are removed; externally managed keys survive.</li></ol><p style="white-space: pre-wrap; margin-top: 0.1em; margin-bottom: 0.2em; unicod e-bidi: plaintext; color: rgb(191, 191, 191); font-family: -apple-system, "system-ui", "Segoe UI", Roboto, sans-serif; font-size: 13px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(18, 19, 20); text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;">Happy either way. (2) seemed the safer default, but (1) may well be intended if <code style="font-family: monospace; color: rgb(140, 140, 140); background: none 0% 0% / auto repeat scroll padding-box border-box rgb(36, 37, 38); padding: 2px 4px; border-radius: 3px; word-break: break-word; font-size: 0.9em;">Parameters</code> is meant to mirror table properties exactly.</p> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
