[GitHub] incubator-guacamole-client pull request #182: GUACAMOLE-363: Add support for...

[mike-jumper <gi...@git.apache.org>]

Github user mike-jumper commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/182#discussion_r137602825
  
    --- Diff: extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-sqlserver/src/main/resources/org/apache/guacamole/auth/jdbc/connection/ConnectionMapper.xml ---
    @@ -0,0 +1,235 @@
    +<?xml version="1.0" encoding="UTF-8" ?>
    +<!DOCTYPE mapper PUBLIC "-//mybatis.org//DTD Mapper 3.0//EN"
    +    "http://mybatis.org/dtd/mybatis-3-mapper.dtd" >
    +
    +<!--
    +    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.
    +-->
    +
    +<mapper namespace="org.apache.guacamole.auth.jdbc.connection.ConnectionMapper" >
    +
    +    <!-- Result mapper for connection objects -->
    +    <resultMap id="ConnectionResultMap" type="org.apache.guacamole.auth.jdbc.connection.ConnectionModel" >
    +
    +        <!-- Connection properties -->
    +        <id     column="connection_id"            property="objectID"              jdbcType="INTEGER"/>
    +        <result column="connection_name"          property="name"                  jdbcType="VARCHAR"/>
    +        <result column="parent_id"                property="parentIdentifier"      jdbcType="INTEGER"/>
    +        <result column="protocol"                 property="protocol"              jdbcType="VARCHAR"/>
    +        <result column="max_connections"          property="maxConnections"        jdbcType="INTEGER"/>
    +        <result column="max_connections_per_user" property="maxConnectionsPerUser" jdbcType="INTEGER"/>
    +        <result column="proxy_hostname"           property="proxyHostname"         jdbcType="VARCHAR"/>
    +        <result column="proxy_port"               property="proxyPort"             jdbcType="INTEGER"/>
    +        <result column="proxy_encryption_method"  property="proxyEncryptionMethod" jdbcType="VARCHAR"
    +                javaType="org.apache.guacamole.net.auth.GuacamoleProxyConfiguration$EncryptionMethod"/>
    +        <result column="connection_weight"        property="connectionWeight"      jdbcType="INTEGER"/>
    +        <result column="failover_only"            property="failoverOnly"          jdbcType="BOOLEAN"/>
    +
    +        <!-- Associated sharing profiles -->
    +        <collection property="sharingProfileIdentifiers" resultSet="sharingProfiles" ofType="java.lang.String"
    +                    column="connection_id" foreignColumn="primary_connection_id">
    +            <result column="sharing_profile_id"/>
    +        </collection>
    +
    +    </resultMap>
    +
    +    <!-- Select all connection identifiers -->
    +    <select id="selectIdentifiers" resultType="string">
    +        SELECT connection_id 
    +        FROM [dbo].[guacamole_connection]
    --- End diff --
    
    It's not that I think we need to require the default schema vs. something else, but rather that I'm surprised that each query would need to specify that schema. I would have expected that the schema would be specified at the JDBC connection level, and would then effect all queries which do not explicitly specify a different schema.
    
    In the case of the PostgreSQL implementation, use of the default `public` schema was due to not specifying the schema at all, which was probably mostly due to my inexperience with PostgreSQL at the time. If something about PostgreSQL then required that I hard-code the `public` schema into all queries, I think I'd start getting worried.
    
    I'm definitely far from an expert in SQL Server, but it feels like hard-coding the schema into each query would be bad. I don't know what the alternative would be. If the expectation for SQL Server databases is that the application itself dictates the schema name, etc. (not just the tables), then perhaps I'm steering things in the wrong direction here by pushing use of the default schema.


---