Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Problem in the description field when using the MariaDB or MySQL #3431

Merged
merged 10 commits into from
Aug 29, 2024

Conversation

MarceloNunesAlves
Copy link
Contributor

This pull request addresses an issue where the 'description' fields, defined as str in the SQLModel model, were being created as varchar(255) columns in MariaDB and MySQL databases. This behavior was inconsistent with the expected text data type for these fields.

Changes:

  • Modified the data type of the 'description' fields in the SQLModel model to sqlalchemy.Text. This ensures that the corresponding columns in MariaDB and MySQL are created as text columns, allowing for larger text values.
  • Note: PostgreSQL is not affected by this change as it does not require a specific size for text columns.

Reasoning:

  • The varchar(255) data type imposes a character limit, which may not be sufficient for storing longer descriptions.
  • Using the text data type provides more flexibility for storing variable-length text data without size constraints.

Below is the print with the error generated:

Screenshot from 2024-08-19 14-08-15

I think it is related to this Issuer that was closed: #1890

log_error_mysql_maria.log

@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Aug 19, 2024
Copy link
Contributor

Pull Request Validation Report

This comment is automatically generated by Conventional PR

Whitelist Report

Whitelist Active Result
Pull request is a draft and should be ignored
Pull request is made by a whitelisted user and should be ignored
Pull request is submitted by a bot and should be ignored
Pull request is submitted by administrators and should be ignored

Result

Pull request does not satisfy any enabled whitelist criteria. Pull request will be validated.

Validation Report

Validation Active Result
All commits in this pull request has valid messages
Pull request does not introduce too many changes
Pull request has a valid title
Pull request has mentioned issues
Pull request has valid branch name
Pull request should have a non-empty body

Result

Pull request satisfies all enabled pull request rules.

Last Modified at 19 Aug 24 18:07 UTC

@dosubot dosubot bot added the bug Something isn't working label Aug 19, 2024
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Aug 19, 2024
Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-3431.dmtpw4p5recq1.amplifyapp.com

@ogabrielluiz
Copy link
Contributor

ogabrielluiz commented Aug 20, 2024

Hey @MarceloNunesAlves

Thanks for this PR. We just need to add the migration script to it.

The steps should be:
cd src/backend/base/langflow && poetry install
then:
poetry run alembic revision --autogenerate -m "Update description columns type"
then, in the newly created file, paste the following code:

"""Update description columns type

Revision ID: 4522eb831f5c
Revises: 0d60fcbd4e8e
Create Date: 2024-08-20 11:15:58.282318

"""

from typing import Sequence, Union

import sqlalchemy as sa
from alembic import op
from langflow.utils import migration
from sqlalchemy.engine.reflection import Inspector

# revision identifiers, used by Alembic.
revision: str = "4522eb831f5c"
down_revision: Union[str, None] = "0d60fcbd4e8e"
branch_labels: Union[str, Sequence[str], None] = None
depends_on: Union[str, Sequence[str], None] = None


def upgrade() -> None:
    conn = op.get_bind()
    # ### commands auto generated by Alembic - please adjust! ###
    inspector = Inspector.from_engine(conn)  # type: ignore

    with op.batch_alter_table("flow", schema=None) as batch_op:
        if migration.column_exists(table_name="flow", column_name="description", conn=conn):
            columns = inspector.get_columns("flow")
            description_column = next((column for column in columns if column["name"] == "description"), None)
            if description_column is not None and isinstance(description_column["type"], sa.VARCHAR):
                batch_op.alter_column(
                    "description", existing_type=sa.VARCHAR(), type_=sa.Text(), existing_nullable=True
                )

    with op.batch_alter_table("folder", schema=None) as batch_op:
        if migration.column_exists(table_name="flow", column_name="description", conn=conn):
            columns = inspector.get_columns("folder")
            description_column = next((column for column in columns if column["name"] == "description"), None)
            if description_column is not None and isinstance(description_column["type"], sa.VARCHAR):
                batch_op.alter_column(
                    "description", existing_type=sa.VARCHAR(), type_=sa.Text(), existing_nullable=True
                )

    # ### end Alembic commands ###


def downgrade() -> None:
    conn = op.get_bind()
    # ### commands auto generated by Alembic - please adjust! ###
    inspector = Inspector.from_engine(conn)  # type: ignore
    with op.batch_alter_table("folder", schema=None) as batch_op:
        if migration.column_exists(table_name="folder", column_name="description", conn=conn):
            columns = inspector.get_columns("folder")
            description_column = next((column for column in columns if column["name"] == "description"), None)
            if description_column is not None and isinstance(description_column["type"], sa.VARCHAR):
                batch_op.alter_column(
                    "description", existing_type=sa.VARCHAR(), type_=sa.Text(), existing_nullable=True
                )

    with op.batch_alter_table("flow", schema=None) as batch_op:
        if migration.column_exists(table_name="flow", column_name="description", conn=conn):
            columns = inspector.get_columns("flow")
            description_column = next((column for column in columns if column["name"] == "description"), None)
            if description_column is not None and isinstance(description_column["type"], sa.VARCHAR):
                batch_op.alter_column(
                    "description", existing_type=sa.VARCHAR(), type_=sa.Text(), existing_nullable=True
                )
    # ### end Alembic commands ###

Make sure everything is working and commit ;)

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Aug 20, 2024
Copy link
Contributor

@ogabrielluiz ogabrielluiz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

we'll add the migrations

…_description_columns_type.py

Co-authored-by: Gabriel Luiz Freitas Almeida <gabriel@langflow.org>
@MarceloNunesAlves
Copy link
Contributor Author

@ogabrielluiz , I think everything is okay now.

Copy link
Contributor

@ogabrielluiz ogabrielluiz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Aug 20, 2024
@MarceloNunesAlves
Copy link
Contributor Author

@ogabrielluiz , Do you know when this is going to be merged?

@ogabrielluiz ogabrielluiz merged commit 10d0336 into langflow-ai:main Aug 29, 2024
27 of 28 checks passed
carlosrcoelho pushed a commit that referenced this pull request Sep 2, 2024
…#3431)

* fix: Problem in the description field when using the MariaDB or MySQL database.

* fix: Problem in the description field when using the MariaDB or MySQL database.

* fix: Add the migration script to update description columns type.

* [autofix.ci] apply automated fixes

* Update src/backend/base/langflow/alembic/versions/1d90f8a0efe1_update_description_columns_type.py

Co-authored-by: Gabriel Luiz Freitas Almeida <gabriel@langflow.org>

---------

Co-authored-by: Marcelo Nunes <marcelo.nunes@nava.com.br>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Gabriel Luiz Freitas Almeida <gabriel@langflow.org>
@xzqxnet0990
Copy link

@ogabrielluiz ogabrielluiz
I meet another problem on mysql, I think the script should also be changed at the beginning script "260dbcc8b680".

`024-09-04 16:27:44,552 - migration.py[line:218] - INFO: Will assume non-transactional DDL.
2024-09-04 16:27:44,564 - migration.py[line:623] - INFO: Running upgrade -> 260dbcc8b680, Adds tables
[09/04/24 16:27:44] ERROR 2024-09-04 16:27:44 - ERROR

  • utils utils.py:127
    - (pymysql.err.OperationalError)
    (1170, "BLOB/TEXT column 'description'
    used in key specification without a
    key length")
    [SQL: CREATE INDEX ix_flow_description
    ON flow (description)]
    (Background on this error at:
    https://sqlalche.me/e/20/e3q8)
    ERROR 2024-09-04 16:27:44 - ERROR - api_server.py:129
    api_server -
    (pymysql.err.OperationalError)
    (1170, "BLOB/TEXT column
    'description' used in key
    specification without a key
    length")
    [SQL: CREATE INDEX
    ix_flow_description ON flow
    (description)]
    (Background on this error at:
    https://sqlalche.me/e/20/e3q8) `

@xzqxnet0990
Copy link

@ogabrielluiz ogabrielluiz I meet another problem on mysql, I think the script should also be changed at the beginning script "260dbcc8b680".

`024-09-04 16:27:44,552 - migration.py[line:218] - INFO: Will assume non-transactional DDL. 2024-09-04 16:27:44,564 - migration.py[line:623] - INFO: Running upgrade -> 260dbcc8b680, Adds tables [09/04/24 16:27:44] ERROR 2024-09-04 16:27:44 - ERROR

  • utils utils.py:127
    • (pymysql.err.OperationalError)
      (1170, "BLOB/TEXT column 'description'
      used in key specification without a
      key length")
      [SQL: CREATE INDEX ix_flow_description
      ON flow (description)]
      (Background on this error at:
      https://sqlalche.me/e/20/e3q8)
      ERROR 2024-09-04 16:27:44 - ERROR - api_server.py:129
      api_server -
      (pymysql.err.OperationalError)
      (1170, "BLOB/TEXT column
      'description' used in key
      specification without a key
      length")
      [SQL: CREATE INDEX
      ix_flow_description ON flow
      (description)]
      (Background on this error at:
      https://sqlalche.me/e/20/e3q8) `

And I fix the code, I think it works.

260dbcc8b680_adds_tables.py line 105
with op.batch_alter_table("flow", schema=None) as batch_op:
    flow_columns = [col["name"] for col in inspector.get_columns("flow")]
    if "user_id" not in flow_columns:
        batch_op.add_column(
            sa.Column(
                "user_id",
                sqlmodel.sql.sqltypes.GUID(),
                nullable=True,  # This should be False, but we need to allow NULL values for now
            )
        )
    if "user.id" not in existing_fks_flow:
        batch_op.create_foreign_key("fk_flow_user_id", "user", ["user_id"], ["id"])
    if "ix_flow_description" not in existing_indices_flow:
        # Corrected way to specify key length for the description column index
        batch_op.create_index(batch_op.f("ix_flow_description"), [sa.text("description(255)")], unique=False)
    if "ix_flow_name" not in existing_indices_flow:
        batch_op.create_index(batch_op.f("ix_flow_name"), ["name"], unique=False)

@xzqxnet0990
Copy link

@ogabrielluiz ogabrielluiz I meet another problem on mysql, I think the script should also be changed at the beginning script "260dbcc8b680".
`024-09-04 16:27:44,552 - migration.py[line:218] - INFO: Will assume non-transactional DDL. 2024-09-04 16:27:44,564 - migration.py[line:623] - INFO: Running upgrade -> 260dbcc8b680, Adds tables [09/04/24 16:27:44] ERROR 2024-09-04 16:27:44 - ERROR

  • utils utils.py:127

    • (pymysql.err.OperationalError)
      (1170, "BLOB/TEXT column 'description'
      used in key specification without a
      key length")
      [SQL: CREATE INDEX ix_flow_description
      ON flow (description)]
      (Background on this error at:
      https://sqlalche.me/e/20/e3q8)
      ERROR 2024-09-04 16:27:44 - ERROR - api_server.py:129
      api_server -
      (pymysql.err.OperationalError)
      (1170, "BLOB/TEXT column
      'description' used in key
      specification without a key
      length")
      [SQL: CREATE INDEX
      ix_flow_description ON flow
      (description)]
      (Background on this error at:
      https://sqlalche.me/e/20/e3q8) `

And I fix the code, I think it works.

260dbcc8b680_adds_tables.py line 105
with op.batch_alter_table("flow", schema=None) as batch_op:
    flow_columns = [col["name"] for col in inspector.get_columns("flow")]
    if "user_id" not in flow_columns:
        batch_op.add_column(
            sa.Column(
                "user_id",
                sqlmodel.sql.sqltypes.GUID(),
                nullable=True,  # This should be False, but we need to allow NULL values for now
            )
        )
    if "user.id" not in existing_fks_flow:
        batch_op.create_foreign_key("fk_flow_user_id", "user", ["user_id"], ["id"])
    if "ix_flow_description" not in existing_indices_flow:
        # Corrected way to specify key length for the description column index
        batch_op.create_index(batch_op.f("ix_flow_description"), [sa.text("description(255)")], unique=False)
    if "ix_flow_name" not in existing_indices_flow:
        batch_op.create_index(batch_op.f("ix_flow_name"), ["name"], unique=False)

I fix it in #3678

@sa411022
Copy link
Contributor

The columns listed below may also need to be changed from VARCHAR(255) to TEXT.

  • message.text
  • variable.value
  • vertex_build.params

@MarceloNunesAlves
Copy link
Contributor Author

Hi @sa411022, how are you? Do you push a PR about this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

getting a column size error when using MySQL.
4 participants