Repository: mlflow/mlflow
Pull request: Increase maximum param value length to 500
<!-- 🚨 We recommend pull requests be filed from a non-master branch on a repository fork (e.g. <username>:fix-xxx). 🚨 -->

## Related Issues/PRs

<!--
Fix for `#6183`. 
Please reference any related feature requests, issues, or PRs here. For example, `#123`. To automatically close the referenced items when this PR is merged, please use a closing keyword (close, fix, or resolve). For example, `Close #123`. See https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue for more information.
-->



## What changes are proposed in this pull request?
Fix #6183. Originally a bug in different validation logic for `log_params` vs `log_param`, this morphed into increasing maximum length of parameter value with implications into backend stores.

- increase validation of maximum param value length from 250 to 500
- unification of validation logic for `log_param` and `log_params`
- change of db model for `SqlAlchemyStore`
- migration script for `SqlAlchemyStore`
- documentation updated for `log_param`


## How is this patch tested?

I have run the same code that initiated the original bug. It is testing the `FileStore` only. Not sure if more rigorous tests are needed?

```python
python repro.py 45
MLflow version: 1.27.1.dev0
Tracking URI:
Artifact URI: file:///Users/xxx/mlflow-bug/mlruns/0/998bb9ab1ad6478bbdc35cde0dcaa116/artifacts
Length of strignified param 260
We want to log {'p': [0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0, 11.0, 12.0, 13.0, 14.0, 15.0, 16.0, 17.0, 18.0, 19.0, 20.0, 21.0, 22.0, 23.0, 24.0, 25.0, 26.0, 27.0, 28.0, 29.0, 30.0, 31.0, 32.0, 33.0, 34.0, 35.0, 36.0, 37.0, 38.0, 39.0, 40.0, 41.0, 42.0, 43.0, 44.0]}
Using `log_param`, parameters was logged:  True
Using `log_params`, parameters was logged:  True
(plain)
```

- [x] I have written tests (not required for typo or doc fix) and confirmed the proposed feature/bug-fix/change works.

<!--
Please describe how you confirmed the proposed feature/bug-fix/change works here. For example, if you fixed an MLflow client API, you could attach the code that didn't work prior to the fix but works now, or if you added a new feature on MLflow UI, you could attach a video that demonstrates the feature.
-->

## Does this PR change the documentation?

- [ ] No. You can skip the rest of this section.
- [x] Yes. Make sure the changed pages / sections render correctly by following the steps below.

1. Check the status of the `ci/circleci: build_doc` check. If it's successful, proceed to the
   next step, otherwise fix it.
2. Click `Details` on the right to open the job page of CircleCI.
3. Click the `Artifacts` tab.
4. Click `docs/build/html/index.html`.
5. Find the changed pages / sections and make sure they render correctly.

## Release Notes

### Is this a user-facing change?

- [ ] No. You can skip the rest of this section.
- [x] Yes. Give a description of this change to be included in the release notes for MLflow users.

Unify maximum length of parameter value to 500 for `FileStore`, `RestStore` and `SqlAlchemyStore`.

### What component(s), interfaces, languages, and integrations does this PR affect?
Components
- [ ] `area/artifacts`: Artifact stores and artifact logging
- [ ] `area/build`: Build and test infrastructure for MLflow
- [x] `area/docs`: MLflow documentation pages
- [ ] `area/examples`: Example code
- [ ] `area/model-registry`: Model Registry service, APIs, and the fluent client calls for Model Registry
- [ ] `area/models`: MLmodel format, model serialization/deserialization, flavors
- [ ] `area/pipelines`: Pipelines, Pipeline APIs, Pipeline configs, Pipeline Templates
- [ ] `area/projects`: MLproject format, project running backends
- [ ] `area/scoring`: MLflow Model server, model deployment tools, Spark UDFs
- [ ] `area/server-infra`: MLflow Tracking server backend
- [x] `area/tracking`: Tracking Service, tracking client APIs, autologging

Interface
- [ ] `area/uiux`: Front-end, user experience, plotting, JavaScript, JavaScript dev server
- [ ] `area/docker`: Docker use across MLflow's components, such as MLflow Projects and MLflow Models
- [x] `area/sqlalchemy`: Use of SQLAlchemy in the Tracking Service or Model Registry
- [ ] `area/windows`: Windows support

Language
- [ ] `language/r`: R APIs and clients
- [ ] `language/java`: Java APIs and clients
- [ ] `language/new`: Proposals for new client languages

Integrations
- [ ] `integrations/azure`: Azure and Azure ML integrations
- [ ] `integrations/sagemaker`: SageMaker integrations
- [ ] `integrations/databricks`: Databricks integrations

<!--
Insert an empty named anchor here to allow jumping to this section with a fragment URL
(e.g. https://github.com/mlflow/mlflow/pull/123#user-content-release-note-category).
Note that GitHub prefixes anchor names in markdown with "user-content-".
-->
<a name="release-note-category"></a>
### How should the PR be classified in the release notes? Choose one:

- [ ] `rn/breaking-change` - The PR will be mentioned in the "Breaking Changes" section
- [ ] `rn/none` - No description will be included. The PR will be mentioned only by the PR number in the "Small Bugfixes and Documentation Updates" section
- [ ] `rn/feature` - A new user-facing feature worth mentioning in the release notes
- [x] `rn/bug-fix` - A user-facing bug fix worth mentioning in the release notes
- [ ] `rn/documentation` - A user-facing documentation change worth mentioning in the release notes


Problem statement:
[BUG] Log_params seems to not use the correct maximum length of value
### Willingness to contribute

Yes. I would be willing to contribute a fix for this bug with guidance from the MLflow community.

### MLflow version

1.27.0

### System information

- **OS Platform and Distribution MacOS 12.1**:
- **Python version 3.8.13**:

### Describe the problem

`log_params` seems to use different validation logic for max. parameter value length compared to `log_param`.
According to doc, parameter value should support length up to 5000 but in fact 250 limit seems to be there.
This passes:
```
some_params= {'p': [float(c) for c in range(int(45))]}
mlflow.log_param('p', some_params['p'])
```

But the same parameter logged with `log_params` will cause error
```
mlflow.log_params(some_params)
```

### Tracking information

```
MLflow version: 1.27.0
Tracking URI: file:///***/mlflow-bug/mlruns
Artifact URI: file:///***/mlflow-bug/mlruns/0/726fe0e42d564b7a93e395cb81fa593f/artifacts
```

### Code to reproduce issue

```python
import mlflow

print("MLflow version:", mlflow.__version__)
print("Tracking URI:", mlflow.get_tracking_uri())
print("Artifact URI:", mlflow.get_artifact_uri())
r=45
some_params= {'p': [float(c) for c in range(int(r))]}
print("Length of stringified param", len((str(some_params['p']))))

mlflow.end_run()
with mlflow.start_run(experiment_id='0'):
    rid2 = mlflow.active_run().info.run_id
    mlflow.log_params(some_params)
```

### Other info / logs

```python-traceback
Traceback (most recent call last):
  File "repro_mini.py", line 13, in <module>
    mlflow.log_params(some_params)
  File "/Users/jan.sindlar/Library/Caches/pypoetry/virtualenvs/mlflow-bug-ZZrSkkBy-py3.8/lib/python3.8/site-packages/mlflow/tracking/fluent.py", line 675, in log_params
    MlflowClient().log_batch(run_id=run_id, metrics=[], params=params_arr, tags=[])
  File "/Users/jan.sindlar/Library/Caches/pypoetry/virtualenvs/mlflow-bug-ZZrSkkBy-py3.8/lib/python3.8/site-packages/mlflow/tracking/client.py", line 918, in log_batch
    self._tracking_client.log_batch(run_id, metrics, params, tags)
  File "/Users/jan.sindlar/Library/Caches/pypoetry/virtualenvs/mlflow-bug-ZZrSkkBy-py3.8/lib/python3.8/site-packages/mlflow/tracking/_tracking_service/client.py", line 315, in log_batch
    self.store.log_batch(
  File "/Users/jan.sindlar/Library/Caches/pypoetry/virtualenvs/mlflow-bug-ZZrSkkBy-py3.8/lib/python3.8/site-packages/mlflow/store/tracking/file_store.py", line 883, in log_batch
    _validate_batch_log_data(metrics, params, tags)
  File "/Users/jan.sindlar/Library/Caches/pypoetry/virtualenvs/mlflow-bug-ZZrSkkBy-py3.8/lib/python3.8/site-packages/mlflow/utils/validation.py", line 320, in _validate_batch_log_data
    _validate_param(param.key, param.value)
  File "/Users/jan.sindlar/Library/Caches/pypoetry/virtualenvs/mlflow-bug-ZZrSkkBy-py3.8/lib/python3.8/site-packages/mlflow/utils/validation.py", line 148, in _validate_param
    _validate_length_limit("Param value", MAX_PARAM_VAL_LENGTH, value)
  File "/Users/jan.sindlar/Library/Caches/pypoetry/virtualenvs/mlflow-bug-ZZrSkkBy-py3.8/lib/python3.8/site-packages/mlflow/utils/validation.py", line 268, in _validate_length_limit
    raise MlflowException(
mlflow.exceptions.MlflowException: Param value '[0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0, 11.0, 12.0, 13.0, 14.0, 15.0, 16.0, 17.0, 18.0, 19.0, 20.0, 21.0, 22.0, 23.0, 24.0, 25.0, 26.0, 27.0, 28.0, 29.0, 30.0, 31.0, 32.0, 33.0, 34.0, 35.0, 36.0, 37.0, 38.0, 39.0, 40.0, 41.0, 42.0, 4' had length 260, which exceeded length limit of 250
```

### What component(s) does this bug affect?

- [ ] `area/artifacts`: Artifact stores and artifact logging
- [ ] `area/build`: Build and test infrastructure for MLflow
- [ ] `area/docs`: MLflow documentation pages
- [ ] `area/examples`: Example code
- [ ] `area/model-registry`: Model Registry service, APIs, and the fluent client calls for Model Registry
- [ ] `area/models`: MLmodel format, model serialization/deserialization, flavors
- [ ] `area/pipelines`: Pipelines, Pipeline APIs, Pipeline configs, Pipeline Templates
- [ ] `area/projects`: MLproject format, project running backends
- [ ] `area/scoring`: MLflow Model server, model deployment tools, Spark UDFs
- [ ] `area/server-infra`: MLflow Tracking server backend
- [X] `area/tracking`: Tracking Service, tracking client APIs, autologging

### What interface(s) does this bug affect?

- [ ] `area/uiux`: Front-end, user experience, plotting, JavaScript, JavaScript dev server
- [ ] `area/docker`: Docker use across MLflow's components, such as MLflow Projects and MLflow Models
- [ ] `area/sqlalchemy`: Use of SQLAlchemy in the Tracking Service or Model Registry
- [ ] `area/windows`: Windows support

### What language(s) does this bug affect?

- [ ] `language/r`: R APIs and clients
- [ ] `language/java`: Java APIs and clients
- [ ] `language/new`: Proposals for new client languages

### What integration(s) does this bug affect?

- [ ] `integrations/azure`: Azure and Azure ML integrations
- [ ] `integrations/sagemaker`: SageMaker integrations
- [ ] `integrations/databricks`: Databricks integrations

Hints from issue discussion:
Thanks for reporting this ! I will help review your fixing PR.
The `log_params` limit the param value length <= 250 at here:
https://github.com/mlflow/mlflow/blob/8693a26bf2acd418aa93aa5ea2e5536caa199fc3/mlflow/utils/validation.py#L148

@dbczumar 

Which param value length limitation shall we use ?

* `log_param` doc say value length limitation is 5000, but the code does not valiate the value length.
* `log_params` validate the value length to be <= 250, otherwise raise error
* `autologging` automatically truncate value to be length <= 250
If I may... I see at least two reasons why not 250: 1) consistency, so `log_params` should allow the user to achieve the same as `log_param`. 2) 250 seems too low, e.g. in the case of parameter vectors of doubles it would only allow very short vectors after stringification.


Yea, 250 is too low, we can consider to increase it. And we should add the same value length limit validation for `log_param`.  @dbczumar WDYT ?
@johnyNJ @WeichenXu123 Thank you for raising this issue and discussing solutions. Here's my proposal:

1. Increase the minimum parameter value length supported by all backends to 500, as documented in https://mlflow.org/docs/latest/rest-api.html#log-param. This will require a database migration for `SqlAlchemyStore`, which current defines `value` as a length-500 column: https://github.com/mlflow/mlflow/blob/2037b384f3cef6c2e8828ec68201040bc3790235/mlflow/store/tracking/dbmodels/models.py#L410. It also requires adjusting length-checking logic in backend validation utilities (https://github.com/mlflow/mlflow/blob/8693a26bf2acd418aa93aa5ea2e5536caa199fc3/mlflow/utils/validation.py#L148), as pointed out by @WeichenXu123.

2. Fix the documentation for `mlflow.log_param()` to reflect that all backend stores support values up to length *500*, instead of *5000*. Also update the REST API documentation (https://mlflow.org/docs/latest/rest-api.html#log-param) to indicate that some backends may support larger parameter values (e.g. third-party backends).

3. Remove all *client-side* validation of parameter length from `log_param()` / `log_params()`, if any is present. Backends should perform all necessary validations.

4. Adjust autologging behavior to only truncate parameters to length-500, rather than length-250.

@johnyNJ @WeichenXu123 Let me know what you think of this proposal. We are open to supporting longer values, though we'd like to understand the use case(s) first.
@dbczumar Good proposal, thank you !
@johnyNJ Would you create PR according to @dbczumar 's proposal above ? 
Or if you have better idea you can also raise it here.
<!-- assign-maintainer -->
@BenWilson2 @dbczumar @harupy @WeichenXu123 Please assign a maintainer and start triaging this issue.
So it looks like I have misunderstood _raison d'être_ of `log_params` a bit. Ever since I became mlflow user, I have been logging parameters with `log_params`. Now I am beginning to understand that `log_params` is, in fact, meant exclusively for hyper-parameters logging while logging of model parameters (ie.e. those estimated by the model) is supposed to be done (indirectly) with `log_model`. Is this understanding correct? If not, then 500 limit will be easily exceeded which is how I found out about this problem. If yes, maybe we can clarify somewhere in doc that params is for hyper-parameters while model.

As for the proposal by @dbczumar above, looks ok and I will have no other comment there.

Commit message: 1. added test for exceeding max length. 2. lint. 3. validation logic addresses null

Signed-off-by: Jan Sindlar <jan.sindlar@eureka.ai>

Patch to review:
diff --git a/mlflow/store/db_migrations/versions/d2999357da94_change_param_value_length_to_500.py b/mlflow/store/db_migrations/versions/d2999357da94_change_param_value_length_to_500.py
new file mode 100644
index 0000000000000..1552e693a7662
--- /dev/null
+++ b/mlflow/store/db_migrations/versions/d2999357da94_change_param_value_length_to_500.py
@@ -0,0 +1,33 @@
+"""change param value length to 500
+
+Revision ID: d2999357da94
+Revises: bd07f7e963c5
+Create Date: 2022-07-27 22:00:13.187596
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+# revision identifiers, used by Alembic.
+revision = "d2999357da94"
+down_revision = "bd07f7e963c5"
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    """
+    Enlarge the maximum param value length to 500.
+    """
+    with op.batch_alter_table("params") as batch_op:
+        batch_op.alter_column(
+            "value",
+            existing_type=sa.String(250),
+            type_=sa.String(500),
+            existing_nullable=False,
+            nullable=False,
+        )
+
+
+def downgrade():
+    pass
diff --git a/mlflow/store/tracking/dbmodels/models.py b/mlflow/store/tracking/dbmodels/models.py
index 3735f1b36b865..9b1d656322033 100644
--- a/mlflow/store/tracking/dbmodels/models.py
+++ b/mlflow/store/tracking/dbmodels/models.py
@@ -416,9 +416,9 @@ class SqlParam(Base):
     """
     Param key: `String` (limit 250 characters). Part of *Primary Key* for ``params`` table.
     """
-    value = Column(String(250), nullable=False)
+    value = Column(String(500), nullable=False)
     """
-    Param value: `String` (limit 250 characters). Defined as *Non-null* in schema.
+    Param value: `String` (limit 500 characters). Defined as *Non-null* in schema.
     """
     run_uuid = Column(String(32), ForeignKey("runs.run_uuid"))
     """
diff --git a/mlflow/store/tracking/file_store.py b/mlflow/store/tracking/file_store.py
index 42e382a07743a..e63a568ab207e 100644
--- a/mlflow/store/tracking/file_store.py
+++ b/mlflow/store/tracking/file_store.py
@@ -38,6 +38,7 @@
 from mlflow.utils.validation import (
     _validate_metric_name,
     _validate_param_name,
+    _validate_param,
     _validate_run_id,
     _validate_tag_name,
     _validate_experiment_id,
@@ -835,7 +836,7 @@ def _writeable_value(self, tag_value):
 
     def log_param(self, run_id, param):
         _validate_run_id(run_id)
-        _validate_param_name(param.key)
+        _validate_param(param.key, param.value)
         run_info = self._get_run_info(run_id)
         check_run_is_active(run_info)
         self._log_run_param(run_info, param)
diff --git a/mlflow/store/tracking/sqlalchemy_store.py b/mlflow/store/tracking/sqlalchemy_store.py
index 7d3303889ea75..eda5684c7a717 100644
--- a/mlflow/store/tracking/sqlalchemy_store.py
+++ b/mlflow/store/tracking/sqlalchemy_store.py
@@ -51,6 +51,7 @@
     _validate_tag,
     _validate_list_experiments_max_results,
     _validate_param_keys_unique,
+    _validate_param,
     _validate_experiment_name,
 )
 from mlflow.utils.mlflow_tags import MLFLOW_LOGGED_MODELS
@@ -872,6 +873,7 @@ def get_metric_history(self, run_id, metric_key):
             return [metric.to_mlflow_entity() for metric in metrics]
 
     def log_param(self, run_id, param):
+        _validate_param(param.key, param.value)
         with self.ManagedSessionMaker() as session:
             run = self._get_run(run_uuid=run_id, session=session)
             self._check_run_is_active(run)
diff --git a/mlflow/tracking/client.py b/mlflow/tracking/client.py
index cd8410d5f566e..1099f01406e53 100644
--- a/mlflow/tracking/client.py
+++ b/mlflow/tracking/client.py
@@ -808,7 +808,7 @@ def log_param(self, run_id: str, key: str, value: Any) -> None:
                     All backend stores support keys up to length 250, but some may
                     support larger keys.
         :param value: Parameter value (string, but will be string-ified if not).
-                      All backend stores support values up to length 250, but some
+                      All backend stores support values up to length 500, but some
                       may support larger values.
 
         .. code-block:: python
diff --git a/mlflow/tracking/fluent.py b/mlflow/tracking/fluent.py
index 9143ce864bc12..b18fc2b8f55e5 100644
--- a/mlflow/tracking/fluent.py
+++ b/mlflow/tracking/fluent.py
@@ -524,7 +524,7 @@ def log_param(key: str, value: Any) -> None:
                 All backend stores support keys up to length 250, but some may
                 support larger keys.
     :param value: Parameter value (string, but will be string-ified if not).
-                  All backend stores support values up to length 250, but some
+                  All backend stores support values up to length 500, but some
                   may support larger values.
 
     .. code-block:: python
diff --git a/mlflow/utils/validation.py b/mlflow/utils/validation.py
index 8d090ea8c9c14..1036606283249 100644
--- a/mlflow/utils/validation.py
+++ b/mlflow/utils/validation.py
@@ -28,7 +28,7 @@
 MAX_METRICS_PER_BATCH = 1000
 MAX_ENTITIES_PER_BATCH = 1000
 MAX_BATCH_LOG_REQUEST_SIZE = int(1e6)
-MAX_PARAM_VAL_LENGTH = 250
+MAX_PARAM_VAL_LENGTH = 500
 MAX_TAG_VAL_LENGTH = 5000
 MAX_EXPERIMENT_TAG_KEY_LENGTH = 250
 MAX_EXPERIMENT_TAG_VAL_LENGTH = 5000
@@ -265,7 +265,7 @@ def _validate_tag_name(name):
 
 
 def _validate_length_limit(entity_name, limit, value):
-    if len(value) > limit:
+    if value is not None and len(value) > limit:
         raise MlflowException(
             "%s '%s' had length %s, which exceeded length limit of %s"
             % (entity_name, value[:250], len(value), limit),
diff --git a/tests/db/schemas/mssql.sql b/tests/db/schemas/mssql.sql
index b7de513583bd9..303541b4e3b02 100644
--- a/tests/db/schemas/mssql.sql
+++ b/tests/db/schemas/mssql.sql
@@ -114,7 +114,7 @@ CREATE TABLE model_version_tags (
 
 CREATE TABLE params (
 	key VARCHAR(250) COLLATE "SQL_Latin1_General_CP1_CI_AS" NOT NULL,
-	value VARCHAR(250) COLLATE "SQL_Latin1_General_CP1_CI_AS" NOT NULL,
+	value VARCHAR(500) COLLATE "SQL_Latin1_General_CP1_CI_AS" NOT NULL,
 	run_uuid VARCHAR(32) COLLATE "SQL_Latin1_General_CP1_CI_AS" NOT NULL,
 	CONSTRAINT param_pk PRIMARY KEY (key, run_uuid),
 	CONSTRAINT "FK__params__run_uuid__45F365D3" FOREIGN KEY(run_uuid) REFERENCES runs (run_uuid)
diff --git a/tests/db/schemas/mysql.sql b/tests/db/schemas/mysql.sql
index 560dd0d7a747e..ecce99e639127 100644
--- a/tests/db/schemas/mysql.sql
+++ b/tests/db/schemas/mysql.sql
@@ -121,7 +121,7 @@ CREATE TABLE model_version_tags (
 
 CREATE TABLE params (
 	key VARCHAR(250) NOT NULL,
-	value VARCHAR(250) NOT NULL,
+	value VARCHAR(500) NOT NULL,
 	run_uuid VARCHAR(32) NOT NULL,
 	PRIMARY KEY (key, run_uuid),
 	CONSTRAINT params_ibfk_1 FOREIGN KEY(run_uuid) REFERENCES runs (run_uuid)
diff --git a/tests/db/schemas/postgresql.sql b/tests/db/schemas/postgresql.sql
index 44c328cfdab75..89927114b5d71 100644
--- a/tests/db/schemas/postgresql.sql
+++ b/tests/db/schemas/postgresql.sql
@@ -119,7 +119,7 @@ CREATE TABLE model_version_tags (
 
 CREATE TABLE params (
 	key VARCHAR(250) NOT NULL,
-	value VARCHAR(250) NOT NULL,
+	value VARCHAR(500) NOT NULL,
 	run_uuid VARCHAR(32) NOT NULL,
 	CONSTRAINT param_pk PRIMARY KEY (key, run_uuid),
 	CONSTRAINT params_run_uuid_fkey FOREIGN KEY(run_uuid) REFERENCES runs (run_uuid)
diff --git a/tests/db/schemas/sqlite.sql b/tests/db/schemas/sqlite.sql
index 23ae7efdda94a..e4a1903500ee2 100644
--- a/tests/db/schemas/sqlite.sql
+++ b/tests/db/schemas/sqlite.sql
@@ -78,8 +78,8 @@ CREATE TABLE runs (
 	experiment_id INTEGER,
 	CONSTRAINT run_pk PRIMARY KEY (run_uuid),
 	FOREIGN KEY(experiment_id) REFERENCES experiments (experiment_id),
-	CONSTRAINT runs_lifecycle_stage CHECK (lifecycle_stage IN ('active', 'deleted')),
 	CONSTRAINT source_type CHECK (source_type IN ('NOTEBOOK', 'JOB', 'LOCAL', 'UNKNOWN', 'PROJECT')),
+	CONSTRAINT runs_lifecycle_stage CHECK (lifecycle_stage IN ('active', 'deleted')),
 	CHECK (status IN ('SCHEDULED', 'FAILED', 'FINISHED', 'RUNNING', 'KILLED'))
 )
 
@@ -122,7 +122,7 @@ CREATE TABLE model_version_tags (
 
 CREATE TABLE params (
 	key VARCHAR(250) NOT NULL,
-	value VARCHAR(250) NOT NULL,
+	value VARCHAR(500) NOT NULL,
 	run_uuid VARCHAR(32) NOT NULL,
 	CONSTRAINT param_pk PRIMARY KEY (key, run_uuid),
 	FOREIGN KEY(run_uuid) REFERENCES runs (run_uuid)
diff --git a/tests/resources/db/latest_schema.sql b/tests/resources/db/latest_schema.sql
index a9f32807e6c0c..ab44b910b22d7 100644
--- a/tests/resources/db/latest_schema.sql
+++ b/tests/resources/db/latest_schema.sql
@@ -122,7 +122,7 @@ CREATE TABLE model_version_tags (
 
 CREATE TABLE params (
 	key VARCHAR(250) NOT NULL,
-	value VARCHAR(250) NOT NULL,
+	value VARCHAR(500) NOT NULL,
 	run_uuid VARCHAR(32) NOT NULL,
 	CONSTRAINT param_pk PRIMARY KEY (key, run_uuid),
 	FOREIGN KEY(run_uuid) REFERENCES runs (run_uuid)
diff --git a/tests/store/tracking/test_file_store.py b/tests/store/tracking/test_file_store.py
index c8f365c1f109d..58ce5ea7ebce9 100644
--- a/tests/store/tracking/test_file_store.py
+++ b/tests/store/tracking/test_file_store.py
@@ -885,6 +885,17 @@ def test_log_param_enforces_value_immutability(self):
         run = fs.get_run(run_id)
         assert run.data.params[param_name] == "value1"
 
+    def test_log_param_max_length_value(self):
+        param_name = "new param"
+        param_value = list(range(500))
+        fs = FileStore(self.test_root)
+        run_id = self.exp_data[FileStore.DEFAULT_EXPERIMENT_ID]["runs"][0]
+        fs.log_param(run_id, Param(param_name, param_value))
+        run = fs.get_run(run_id)
+        assert run.data.params[param_name] == str(param_value)
+        with pytest.raises(MlflowException, match="exceeded length"):
+            fs.log_param(run_id, Param(param_name, list(range(1000))))
+
     def test_weird_metric_names(self):
         WEIRD_METRIC_NAME = "this is/a weird/but valid metric"
         fs = FileStore(self.test_root)
@@ -1135,6 +1146,21 @@ def _create_run(self, fs):
             experiment_id=FileStore.DEFAULT_EXPERIMENT_ID, user_id="user", start_time=0, tags=[]
         )
 
+    def test_log_batch_max_length_value(self):
+        param_entities = [Param("long param", list(range(500))), Param("short param", "xyz")]
+        expected_param_entities = [
+            Param("long param", str(list(range(500)))),
+            Param("short param", "xyz"),
+        ]
+        fs = FileStore(self.test_root)
+        run = self._create_run(fs)
+        fs.log_batch(run.info.run_id, (), param_entities, ())
+        self._verify_logged(fs, run.info.run_id, (), expected_param_entities, ())
+
+        with pytest.raises(MlflowException, match="exceeded length"):
+            param_entities = [Param("long param", list(range(1000))), Param("short param", "xyz")]
+            fs.log_batch(run.info.run_id, (), param_entities, ())
+
     def test_log_batch_internal_error(self):
         # Verify that internal errors during log_batch result in MlflowExceptions
         fs = FileStore(self.test_root)
diff --git a/tests/store/tracking/test_sqlalchemy_store.py b/tests/store/tracking/test_sqlalchemy_store.py
index a8d96dd7440f4..8bc4579398c18 100644
--- a/tests/store/tracking/test_sqlalchemy_store.py
+++ b/tests/store/tracking/test_sqlalchemy_store.py
@@ -1058,6 +1058,17 @@ def test_log_null_param(self):
             self.store.log_param(run.info.run_id, param)
         assert exception_context.exception.error_code == ErrorCode.Name(BAD_REQUEST)
 
+    def test_log_param_max_length_value(self):
+        run = self._run_factory()
+        tkey = "blahmetric"
+        tval = "x" * 500
+        param = entities.Param(tkey, tval)
+        self.store.log_param(run.info.run_id, param)
+        run = self.store.get_run(run.info.run_id)
+        assert run.data.params[tkey] == str(tval)
+        with pytest.raises(MlflowException, match="exceeded length"):
+            self.store.log_param(run.info.run_id, entities.Param(tkey, "x" * 1000))
+
     def test_set_experiment_tag(self):
         exp_id = self._experiment_factory("setExperimentTagExp")
         tag = entities.ExperimentTag("tag0", "value0")
@@ -2063,6 +2074,16 @@ def test_log_batch_null_metrics(self):
             self.store.log_batch(run.info.run_id, metrics=metrics, params=[], tags=[])
         assert exception_context.exception.error_code == ErrorCode.Name(INVALID_PARAMETER_VALUE)
 
+    def test_log_batch_params_max_length_value(self):
+        run = self._run_factory()
+        param_entities = [Param("long param", "x" * 500), Param("short param", "xyz")]
+        expected_param_entities = [Param("long param", "x" * 500), Param("short param", "xyz")]
+        self.store.log_batch(run.info.run_id, [], param_entities, [])
+        self._verify_logged(self.store, run.info.run_id, [], expected_param_entities, [])
+        with pytest.raises(MlflowException, match="exceeded length"):
+            param_entities = [Param("long param", "x" * 1000)]
+            self.store.log_batch(run.info.run_id, [], param_entities, [])
+
     def test_upgrade_cli_idempotence(self):
         # Repeatedly run `mlflow db upgrade` against our database, verifying that the command
         # succeeds and that the DB has the latest schema


Code review comments:
diff_hunk: @@ -1135,6 +1146,21 @@ def _create_run(self, fs):
             experiment_id=FileStore.DEFAULT_EXPERIMENT_ID, user_id="user", start_time=0, tags=[]
         )
 
+    def test_log_batch_max_length_value(self):
+        param_entities = [Param("long param", list(range(500))), Param("short param", "xyz")]
path: tests/store/tracking/test_file_store.py
text: @user1:
```suggestion
        param_entities = [Param("long param", "x" * 500), Param("short param", "xyz")]
```

diff_hunk: @@ -885,6 +885,17 @@ def test_log_param_enforces_value_immutability(self):
         run = fs.get_run(run_id)
         assert run.data.params[param_name] == "value1"
 
+    def test_log_param_max_length_value(self):
+        param_name = "new param"
+        param_value = list(range(500))
+        fs = FileStore(self.test_root)
+        run_id = self.exp_data[FileStore.DEFAULT_EXPERIMENT_ID]["runs"][0]
+        fs.log_param(run_id, Param(param_name, param_value))
+        run = fs.get_run(run_id)
+        assert run.data.params[param_name] == str(param_value)
path: tests/store/tracking/test_file_store.py
text: @user1:
```suggestion
        assert run.data.params[param_name] == param_value
```

diff_hunk: @@ -885,6 +885,17 @@ def test_log_param_enforces_value_immutability(self):
         run = fs.get_run(run_id)
         assert run.data.params[param_name] == "value1"
 
+    def test_log_param_max_length_value(self):
+        param_name = "new param"
+        param_value = list(range(500))
path: tests/store/tracking/test_file_store.py
text: @user1:
```suggestion
        param_value = "x" * 500
```

diff_hunk: @@ -1135,6 +1146,21 @@ def _create_run(self, fs):
             experiment_id=FileStore.DEFAULT_EXPERIMENT_ID, user_id="user", start_time=0, tags=[]
         )
 
+    def test_log_batch_max_length_value(self):
+        param_entities = [Param("long param", list(range(500))), Param("short param", "xyz")]
+        expected_param_entities = [
+            Param("long param", str(list(range(500)))),
path: tests/store/tracking/test_file_store.py
text: @user1:
```suggestion
            Param("long param", "x" * 500),
```

diff_hunk: @@ -885,6 +885,17 @@ def test_log_param_enforces_value_immutability(self):
         run = fs.get_run(run_id)
         assert run.data.params[param_name] == "value1"
 
+    def test_log_param_max_length_value(self):
+        param_name = "new param"
+        param_value = list(range(500))
+        fs = FileStore(self.test_root)
+        run_id = self.exp_data[FileStore.DEFAULT_EXPERIMENT_ID]["runs"][0]
+        fs.log_param(run_id, Param(param_name, param_value))
+        run = fs.get_run(run_id)
+        assert run.data.params[param_name] == str(param_value)
+        with pytest.raises(MlflowException, match="exceeded length"):
+            fs.log_param(run_id, Param(param_name, list(range(1000))))
path: tests/store/tracking/test_file_store.py
text: @user1:
```suggestion
            fs.log_param(run_id, Param(param_name, "x" * 1000))
```

@author:
yes, at this point, everything should have been stringified. Will change

diff_hunk: @@ -1135,6 +1146,21 @@ def _create_run(self, fs):
             experiment_id=FileStore.DEFAULT_EXPERIMENT_ID, user_id="user", start_time=0, tags=[]
         )
 
+    def test_log_batch_max_length_value(self):
+        param_entities = [Param("long param", list(range(500))), Param("short param", "xyz")]
+        expected_param_entities = [
+            Param("long param", str(list(range(500)))),
+            Param("short param", "xyz"),
+        ]
+        fs = FileStore(self.test_root)
+        run = self._create_run(fs)
+        fs.log_batch(run.info.run_id, (), param_entities, ())
+        self._verify_logged(fs, run.info.run_id, (), expected_param_entities, ())
+
+        with pytest.raises(MlflowException, match="exceeded length"):
+            param_entities = [Param("long param", list(range(1000))), Param("short param", "xyz")]
path: tests/store/tracking/test_file_store.py
text: @user1:
```suggestion
            param_entities = [Param("long param", "x" * 1000), Param("short param", "xyz")]
```