Skip to content

Issue 52900: LKSM: Moving a sample to another folder breaks other references to shared files#6699

Merged
XingY merged 10 commits intodevelopfrom
fb_fileIssues256
Jun 3, 2025
Merged

Issue 52900: LKSM: Moving a sample to another folder breaks other references to shared files#6699
XingY merged 10 commits intodevelopfrom
fb_fileIssues256

Conversation

@XingY
Copy link
Contributor

@XingY XingY commented May 27, 2025

Rationale

Issue 52900: LKSM: Moving a sample to another folder breaks other references to shared files
Issue 53070: Moving files could result in duplicate exp.data records
Issue 53178: SQL generation problem from TableUpdaterFileListener

Related Pull Requests

Changes

Issue 52900: LKSM: Moving a sample to another folder breaks other references to shared files
Issue 53070: Moving files could result in duplicate exp.data records

public String getSourceSelect(SqlDialect sqlDialect)
{
String schema = sqlDialect.makeLegalIdentifier(_table.getSchema().getName());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@labkey-matthewb Any suggestion how to avoid using the deprecated makeLegalIdentifier here is appreciated.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed. It is helpful when deprecating methods to give indication as to what should be used/considered instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't really have anything to do with SQLFragment and identifiers, if we're just trying to create a string literal to select. Just create the string you want and use appendValue() instead of append()

Copy link
Contributor

Choose a reason for hiding this comment

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

Are using this selected string to generate SQL later (or is this just for logging/display)? If so it's probably easier to select the schema and table name separately.

selectFrag.append(" NULL AS SourceKey,\n");

//selectFrag.append(" ? AS SourceName\n").add(getName());
selectFrag.append(" ").append(_table.getSchema().getSqlDialect().getStringHandler().quoteStringLiteral(getSourceName())).append(" AS SourceName\n");
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not exactly sure what's going on here (selecing a bit of SQL as a string value?), but can we just do .appendValue(getSourceName())?


public String getSourceSelect(SqlDialect sqlDialect)
{
String schema = sqlDialect.makeLegalIdentifier(_table.getSchema().getName());
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't really have anything to do with SQLFragment and identifiers, if we're just trying to create a string literal to select. Just create the string you want and use appendValue() instead of append()

String schema = sqlDialect.makeLegalIdentifier(_table.getSchema().getName());
String table = sqlDialect.makeLegalIdentifier(_table.getName());
String column = sqlDialect.makeLegalIdentifier(_pathColumn.getName());
return sqlDialect.getStringHandler().quoteStringLiteral(schema + "." + table + "." + column);
Copy link
Contributor

@labkey-matthewb labkey-matthewb May 27, 2025

Choose a reason for hiding this comment

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

Maybe use

// for Db schema table info
return table.getSelectName() + "." + LabKeySql.quoteString(_pathColumn.getName())

// for UserSchema
return table.getUserSchema().getSchemaPath().toSQLString() + "." + LabKeySql.quoteString(_table.getName()) + "." + LabkeySql.quoteString(_pathColumn.getName()

and then appendValue() as mentioned above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. This works.

@XingY XingY merged commit f2871a6 into develop Jun 3, 2025
9 checks passed
@XingY XingY deleted the fb_fileIssues256 branch June 3, 2025 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants