codeant-ai-for-open-source[bot] commented on code in PR #36805: URL: https://github.com/apache/superset/pull/36805#discussion_r2692526114
########## superset/db_engine_specs/greenplum.py: ########## @@ -0,0 +1,30 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from superset.db_engine_specs.postgres import PostgresEngineSpec + + +class GreenplumEngineSpec(PostgresEngineSpec): + """Engine spec for VMware Greenplum (formerly Pivotal Greenplum) + + Greenplum is a massively parallel processing (MPP) database built on PostgreSQL. + It inherits PostgreSQL's SQL syntax and most features. + """ + + engine = "greenplum" + engine_name = "Greenplum" + default_driver = "psycopg2" Review Comment: **Suggestion:** Because this class subclasses the PostgreSQL engine spec without overriding `metadata`, any code that prefers `spec.metadata` over `DATABASE_DOCS` (as indicated in `lib.py`) will see PostgreSQL's description, logo, connection string, and package info for Greenplum, causing incorrect database documentation and UI metadata; defining Greenplum-specific `metadata` on this spec fixes the mismatch. [logic error] <details> <summary><b>Severity Level:</b> Critical 🚨</summary> ```mdx - ❌ Generated DB docs show Postgres info for Greenplum - ⚠️ Docs overview table lists wrong connection string - ⚠️ README logos update may show incorrect asset - ⚠️ UI metadata panels display wrong driver/package info ``` </details> ```suggestion from superset.db_engine_specs.base import DatabaseCategory from superset.db_engine_specs.postgres import PostgresEngineSpec class GreenplumEngineSpec(PostgresEngineSpec): """Engine spec for VMware Greenplum (formerly Pivotal Greenplum) Greenplum is a massively parallel processing (MPP) database built on PostgreSQL. It inherits PostgreSQL's SQL syntax and most features. """ engine = "greenplum" engine_name = "Greenplum" default_driver = "psycopg2" metadata = { "description": "VMware Greenplum is a massively parallel processing (MPP) database built on PostgreSQL.", "logo": "greenplum.png", "homepage_url": "https://greenplum.org/";, "category": DatabaseCategory.TRADITIONAL_RDBMS, "pypi_packages": ["sqlalchemy-greenplum", "psycopg2"], "connection_string": "greenplum://{username}:{password}@{host}:{port}/{database}", "default_port": 5432, "parameters": { "username": "Database username", "password": "Database password", "host": "Greenplum coordinator host", "port": "Default 5432", "database": "Database name", }, "docs_url": "https://docs.vmware.com/en/VMware-Greenplum/";, } ``` <details> <summary><b>Steps of Reproduction ✅ </b></summary> ```mdx 1. Open the PR codebase and observe Greenplum spec at superset/db_engine_specs/greenplum.py: see class GreenplumEngineSpec defined (file lines ~18-30) that subclasses PostgresEngineSpec but does not set a metadata attribute. 2. Run the docs generator introduced by the PR: docs/scripts/generate-database-docs.mjs (PR description says it reads DATABASE_DOCS in superset/db_engine_specs/lib.py). If the docs generator or downstream UI lookup prefers engine-spec.metadata when present, it will fallback to the parent PostgresEngineSpec metadata because Greenplum has no override. 3. Trigger generation of database pages: cd docs && yarn generate:database-docs (per PR Usage). The generator will produce JSON/MDX for Greenplum using Postgres metadata (name, logo, connection string, packages) instead of Greenplum-specific values. 4. Inspect the generated page at docs/docs/databases/greenplum.mdx (or the generated JSON in docs build output). You will observe Postgres connection examples/logo/docs referenced for Greenplum, demonstrating the incorrect metadata was used. Note: This reproduction traces real files added/modified in the PR: superset/db_engine_specs/greenplum.py (class), superset/db_engine_specs/lib.py (DATABASE_DOCS source described in PR), and docs/scripts/generate-database-docs.mjs (generator). The issue occurs because GreenplumEngineSpec inherits PostgresEngineSpec without providing its own metadata, so any code path that reads spec.metadata (rather than DATABASE_DOCS) will surface Postgres data for Greenplum. ``` </details> <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset/db_engine_specs/greenplum.py **Line:** 18:30 **Comment:** *Logic Error: Because this class subclasses the PostgreSQL engine spec without overriding `metadata`, any code that prefers `spec.metadata` over `DATABASE_DOCS` (as indicated in `lib.py`) will see PostgreSQL's description, logo, connection string, and package info for Greenplum, causing incorrect database documentation and UI metadata; defining Greenplum-specific `metadata` on this spec fixes the mismatch. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. ``` </details> -- 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]
