Multi-schema support in V3 must support square brackets in the schema name

[mauroservienti]

taskforce: @kbaley @tmasternak @weralabaj

Problem

In V3 we moved the schema name from the connection string to the endpoint address in the form of my.endpoint@schema. We want to be prepared to support specifying not only schema but also database if we wish to provide multi-database support in the future. In other words we want users to be able to specify address in form my.endpoint@[database].[schema]. To be able to support such format it would be preferable to have canonical (used internally and on the wire) representation of address in SqlServer transport in form my.endpoint@[database].[schema] and my.endpoint@[schema].

Why Core V6

If we leave the canonical transport address as it is right now we would be not wire compatible between v 3.0 and v 3.X with multi-database support. If v 3.X would sent ReplyTo header in form my.endpoint@[schema] v 3.0 could not properly parse it. What it means is that we need add the [] in v 3.0 to enable proper multi-database implementation later.

In Scope of this issue

General rule:

• Whenever users can specify schema (code or message mapping in config file) we support [] and bracket-less syntax. For example the two following samples would have the same result

  meesageType => "Endpoint@dbo" meesageType => "Endpoint@[dbo]"

  configuration.UseTransport().DefaultSchema("dbo"); configuration.UseTransport().DefaultSchema("[dbo]");

• Whenever we pass the transport address to use code we use canonical name with []. For example

  configuration.UserTransport().DefaultSchema("dbo"); configuration.UseTransport().EnableLagacyMultiInstanceMode(ta => ..) //Here ta = "endpoint@[dbo]

  Not in scope for this issue

When we provide multi-database support we should extend the rules for new methods if we add them. For example we might have DefaultDatabase method which should support [] and []-less as well

Plan of Attack

• [x] Implement bracket support in schema
• [x] Update docs => docs will be updated as part of https://github.com/Particular/NServiceBus.SqlServer/issues/139 by @weralabaj
• [x] Update ProductionTests

/cc @tmasternak @ramonsmits Inception: https://github.com/Particular/CustomerSuccess/issues/166

[weralabaj]

@mauroservienti does it mean we should support the square brackets version ONLY? or both?

[tmasternak]

Yes. I think that is the idea. So current Endpoint@dbo would become Endpoint@[dbo]. What that will enable us in future (maybe 3.x) is to extend that with Endpoint@[DatabaseA].[nondbo] which becomes sql-like syntax and enables us easy parsing of db and schema parts.

[weralabaj]

Hm another approacfh would be to assume that if there are no square brackets then it's a single part schema name. I guess it's more important in case of code api, e.g. UseSchema("receiver") vs UseSchema("[receiver]").

From the user perspective I think both could be valid.

[mauroservienti]

it is more complex to parse however I agree with @weralabaj that we should support both with and without brackets, so that we are fully aligned with what SQL supports

[tmasternak]

I agree we should support dbo and [dbo] both in code and in message mappings. In the future when we add multi-database support:

• we provide configuration method .DefaultDatabase()
• if we an endpoint transport address in message mappings in form of [].[] we treat it as db+schema otherwise we treat it only as schema.

That would mean that this can be done as part of multi-db and postponed until after V3 + V6 of Core are out. Thoughts?

[mauroservienti]

if we start by enforcing the usage of brackets we can always relax constraints and in a minor release add support for other syntax styles.

[tmasternak]

What I meant was: Do we need to provide [] support now? Is there any risk in leaving the things are they are now and adding support for [] and [].[] later on?

[mauroservienti]

I sincerely don't know, it depends on how many customers may have "strange" schema names

[tmasternak]

@mauroservienti I've updated the description. I think that backwards compatibility is an argument that makes it Core V6.

[mauroservienti]

:+1:

[weralabaj]

When we specify [database] is it the name of connection string in config file? Asking because apart from db name we'd need credentials etc., so I wonder where all other pieces of information would be.

[mauroservienti]

It is the catalog name @weralabaj, the connection string will be defined with a default database or we no database at all, and the catalog can be specified in the address to override the default value in the connection string.

[weralabaj]

@kbaley are you working on it? Can you please link PR?

[kbaley]

@weralabaj I've started on it. I'll link the PR when I have one. Glad about the conversation here because I thought it was an entirely different issue. SQL Server allows square brackets in their object names. E.g. My]Schema is a valid schema name and "moo" and "[moo]" are considered entirely different schemas. Oddly, SQL Server escapes only the ] in its queries. So if you have a schema named This[is]Schema and a table named [MyTable], you'd query it like so:

SELECT * FROM [This[is]]Schema].[[MyTable]]]

This will affect the endpoint names as well but after talking with @mauroservienti (and assuming I didn't misunderstand), initially we'll put the onus on the user to make sure the schema/table name is properly escaped.

[kbaley]

With :point_up: in mind, is this still accurate:

For example the two following samples would have the same result

configuration.UseTransport().DefaultSchema("dbo"); configuration.UseTransport().DefaultSchema("[dbo]");

[mauroservienti]

initially we'll put the onus on the user to make sure the schema/table name is properly escaped.

yes. It's less effort on our end for the initial release.

[weralabaj]

@kbaley @tmasternak tried to cover all the examples we could think of, but if you find some missing edge case add it to the description, please.

[kbaley]

@weralabaj My question is: If SQL Server treats "dbo" and "[dbo]" as distinct schemas, shouldn't we as well? It seems to me, we should not treat these statements as being equal:

configuration.UseTransport().DefaultSchema("dbo"); configuration.UseTransport().DefaultSchema("[dbo]");

Internally, we should use the [ ] syntax on the endpoint components but I believe we should recommend to people that they not include square brackets in the schema name unless they are actually part of the schema.

[weralabaj]

@mauroservienti @tmasternak ? I don't have strong opinion on that.

[mauroservienti]

Well I see 3 scenarios, based on my SQL/T-SQL limited knowledge (and maybe @ramonsmits cann add some other):

• schema name is a perfectly valid schema name, such as, my, in this case square brackets are not required
• schema name contains invalid character or is a reserved T-SQL keyword, such as, my.schema, in this case square brackets are required
• schema name contains square brackets, such as, [myschema] or worse mysch]ema, in this case square brackets are required and must be escaped

IMO providing a valid, compliant with SQL Server expectation, schema name is a up to the user, we should not validate that schema name and trust the user.

What we need to be sure is that we correctly parse the schema name in light of multi-catalog support, where the endpoint address will end up being: my.endpoint@[my.sche]]ma].[my catalog]

From our perspective it is very important that we are able to split the above address extracting correctly the schema name and the catalog name, however maybe right now is not that important.

[tmasternak]

@kbaley I like the idea to follow the SQL approach. In other words the schema provided by the users will always be put in [] in any expression we execute. As @mauroservienti pointed out it gets more complicated when we start allowing users to provide both catalog and schema in single expression. In such case the question is what should we do with my.endpoint@foo.bar? Should we treat foo.bar as schema and require explicit brackets ([foo].[bar]) to treat it as catalog.schema? Or should we be more relaxed? Personally I would go with more relaxed approach but I think that we can figure out the details when we start adding mulit-catalog and treat it as out of scope for this issue.

[kbaley]

If we go with a more relaxed approach, what is left to do for this issue?

[tmasternak]

@kbaley I think that of we go with 'always putting brackets around user input' approach we need to make sure that AddressProvider always returns bracket version of the address.

[tmasternak]

The second place is ToEnpointAddress on TransportInfrastructure

[ramonsmits]

@tmasternak @mauroservienti my.endpoint@foo.bar should in my opinion not result in `[foo].[bar].[my.endpoint]. If that is what the user wants then let the user specify these values.

[ramonsmits]

Isn't it a possibilty to just allow native syntax in the message mapping? Is there a reason we could not just allow something like:

<MessageEndpointMappings>
    <add Assembly="assembly" Endpoint="[my.catalog][my.schema][my.queue]" />
</MessageEndpointMappings>

That would be the most readable and allow any kind of name the customer would want to use.

[mauroservienti]

It breaks wire compatibility

[tmasternak]

@kbaley do you think it would be beneficial to have a sync and go through plan of attack and potentially share work?

[kbaley]

@tmasternak Yes definitely. I have a tenuous grasp on the issues so I'll need some clarification

[tmasternak]

After quick sync with @kbaley we decided that we will start with supporting both bracket and bracket-less syntax for schema:

meesageType => "Endpoint@dbo" meesageType => "Endpoint@[dbo]"

Internally all string representations of addresses will have brackets. On the implementation level this will result in changing Parse and ToString logic of QueueAddress.

In the future when we support multi-catalog we will support brackets syntax [catalog].[dbo] only.

[weralabaj]

@tmasternak can that issue be closed?

[tmasternak]

@weralabaj no we still need to update the production tests to the newest version.