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

feat: add a new annotation @Sql #3000

Closed

Conversation

Lambert-Rao
Copy link

I add an @Sql annotation to allow user reuse Sql statement in Mapper interface.

This example shows using @Sql annotation to define a statement that can be reuse in <include> :

@Sql(id = "selectWithIfId", value = "<script><if test='id != null'>id</if><if test='name != null'>name</if></script>")
void provideSql();

@Select("<script>select<include refid='selectWithIfId'/> from test</script>")
String selectWithIncludeAndIf(Integer id, String name);

reference to issue #2986

@harawata
Copy link
Member

harawata commented Nov 7, 2023

Hello @Lambert-Rao ,

I apologize if I wasn't clear enough, but (I thought) I wrote that we do not plan to support it at the moment.

We are deliberate in adding new features to keep the project simple.
As I wrote, the XML <sql> works fine and we usually do not add an extra code just because it is 'inconvenient' for a user.

@Lambert-Rao
Copy link
Author

Okay, thanks for your reply, @harawata.

I'm sorry to disturb you again, but we do need this feature for reasons:

  1. Annotation is much easier to use than XML, I think it's a great feature of mybatis to use annotations to free us from writing XMLs. Our team maintain many projects, but XML file has a high maintenance cost, so we really love to use annotations instead of using mapper.xml.

  2. Sometimes, we need to split a mapper.xml file into multiple xml files when code refactoring, which is also a difficult task for a new programmer in team, the same as point 1, annotation is easier to use and maintain(in this point , for code refactoring ).

  3. When we need to use some repeatable String (such as table name), it's not an elegant way to use static String like this : @Select (["select * from", TABLE_STRING, "where id = #{id}"]), we prefer to use a single String like this: @Select ("<script> select * from <include refid='tableName'/> where id = #{id} </script>"). I think a single String can provide more readability than using String[]. Also, we want to use java textblock to replace long annotation with String[], please read flowing examples.

multiple line annotation with String[]:

@Update({"<script>",
            "update",
         	RESOURCE_TABLE,
            "<set>",
            "<if test = '#{roleId} != null'>role_id=#{roleId},</if>",
            "<if test = '#{roleName} != null'>role_name=#{roleName},</if>",
            "<if test = '#{roleDescription} != null'>role_description=#{roleDescription},</if>",
            "</set>",
            "where role_id = #{roldId}",
            "</script>"
    })

multiple line annotation with text block :

@Update("""
        <script>
        update
        <include refid="RESOURCE_TABLE"/>
        <set>
        <if test = '#{roleId} != null'>role_id=#{roleId},</if>
        <if test = '#{roleName} != null'>role_name=#{roleName},</if>
        <if test = '#{roleDescription} != null'>role_description=#{roleDescription},</if>
        </set>
        where role_id = #{roldId}
         </script>
        """
       )

if we don't add @Sql feature, we need to use the first solution with many " and ,, I think the second solution is more readable.

  1. Our code will not break original design or add complexity, could you take some time reviewing our code, we use proxy mode to add this feature, I think it will not increase coupling.
  2. Our team has an experienced developer(Apache commiter), who are deeply involved in open source, we can guarantee the quality of design and code. We will continue to follow up and make improvements based on user feedback.

Sincerely thank you for taking time reading my explanation, please considering review out code. We do need this feature, and we are glad that we can participate in this excellent project.

@harawata
Copy link
Member

harawata commented Nov 8, 2023

The limitation (i.e. unable to use <sql>) is specific to your organization and the proposed change is not trivial.
We might reconsider in a future as it's always a possibility, but at this point, the answer is 'no'.
And, just to be clear, I'm not cliticizing the PRs quality in any way.


I honestly don't understand what's so hard about maintaining XML, but if you so desperately want to avoid it, you should consider using SQL providers.

Just to give you an idea, you can add custom annotations to your mapper classes/methods.

@MyVar(id = "__COLUMNS__", value = "id, name, ...")
@MyVar(id = "__TABLE__", value = "student")
public interface StudentMapper {

  @SelectProvider // assuming defaultSqlProviderType is specified
  @Lang(XMLLanguageDriver.class)
  @MySql("select __COLUMNS__ from __TABLE__ where id = #{id}")
  Student selectStudentById(Integer id);

  @InsertProvider // assuming defaultSqlProviderType is specified
  @Lang(XMLLanguageDriver.class)
  @MySql("insert into __TABLE__ (__COLUMNS__) values (#{id}, #{name})")
  int insert(Student student);

}

And when the statement is called, the provider performs the variable substitution.

public class SubstrProvider implements ProviderMethodResolver {
  @Override
  public Method resolveMethod(ProviderContext context) {
    return this.getClass().getDeclaredMethod("substr", ProviderContext.class);
  }

  public String substr(ProviderContext context) {
    String sql = // get value of @MySql
    Map<String, String> myVars = // collect id/value of @MyVar
    // replace variables in the sql
    return "<script>" + finalSql + "</script>";
  }
}

For the provider implementation (accessing annotations, etc.), there are some examples in this test.
Regarding the variable replacement, a tool like StringSubstitutor might come in handy.

If you have difficulty with provider implementation, I would be happy to help, but it might take a while for me to reply.

@githublaohu
Copy link

@harawata
As a manager, I hope the less intrusion the better. Introducing Provider annotations is too intrusive. And it's not comprehensive enough。

There are many solutions, and how to make them lightweight, simple, comprehensive,and easy to use is the most important.I hope the official can provide a solution, which will reduce maintenance costs, learning costs, management costs, and migration costs

Developers don't even want a demo

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